OpenProcessing.org JavaScript program - Building a list of matrices

Question

My openprocessing.org JavaScript code listed below produces the output I want but doesn’t seem very elegant to me especially the ".M" in "matrices[mat].M[row][col]". Any comments to improve the quality of my code would be appreciated.

The purpose of this code snippet is so I can learn how to appropriately deal with a list of matrices. Eventually I hope to write a program that computes 1344 5x5 Latin squares and store them all in one variable.

The following correct output is what the program produces:

Three 3x3 matrices and their sum=81

let matrices = [];
function setup() {
 createCanvas(windowWidth, windowHeight);
 
 matrices.push(new Matrix([ [0,1,2], [1,2,3], [2,3,4] ]));
 matrices.push(new Matrix([ [1,2,3], [2,3,4], [3,4,5] ]));
 matrices.push(new Matrix([ [2,3,4], [3,4,5], [4,5,6] ]));
}
function draw() {
 
 background('black');
 fill('white');
 sum=0;
 for( mat=0; mat<matrices.length; mat++ ) {
 for( row=0; row<3; row++ ) {
 for( col=0; col<3; col++ ) {
 sum += matrices[mat].M[row][col];
 textSize(20);
 text( matrices[mat].M[row][col], 500+20*col, 100+120*mat+30*row);
 }
 }
 }
 
 textSize(30);
 text( 'Sum=', 500, 500 );
 text( sum, 500, 540 );
}
class Matrix {
 
 constructor(m) {
 this.M = m;
 }
 
}

Answer 1

Good start! I like the idea of rendering these matrices. The code is straightforward and readable and your variable names are generally good.

Remarks:

• Always scope variables with let or const. Never use bare variables like mat=0 without let to scope it to the block. If you have two variables with the same name, or accidentally access a variable when it shouldn't exist, bugs often ensue. Try to tightly scope state as much as possible to reduce complexity.

• The Matrix class doesn't accomplish much, since it consists of a single variable and no functions at the moment. Perhaps soon you'll add operations to it, but for now, a matrix can simply be a variable. Classes are primarily useful when you want to group a bunch of related variables and functions together, and establish a set of rules for accessing and manipulating that state.

  Once you do add a class, you'll probably not want to let the user of the class access the .M property directly, or they could mess up state within the class.

• Avoid counter-based for loops unless necessary. Prefer for ... of loops; they look cleaner and are less prone to off-by-one bugs. Caveat: sometimes, for performance reasons, for loops are acceptable, but I'd avoid premature optimization.

• Use prettier to format your code in a way that reduces the friction for yourself and other JS programmers to read your code.

• Since you never reassign let matrices = [];, you can use const matrices = [];. This avoids an accidental reassignment and more accurately communicates your intent.

• Since matrices is unrelated to P5 you can move it out of the setup function and simply declare it once, without modifying the array with push.

• textSize(20); is always the same in the middle of the loops, so you can save some CPU cycles by moving it outside of the loops so it runs once per frame.

• Code like text( matrices[mat].M[row][col], 500+20*col, 100+120*mat+30*row); is difficult to read because there are many operations on one line and no named parameters. You can move the arguments out to named variables, value, x and y, then use text(value, x, y). Another trick I like to use for calls like this is to spread it across multiple lines, with comments to denote each parameter by name:

  text(
   matrices[mat].M[row][col],
   500 + 20 * col, // x
   100 + 120 * mat + 30 * row // y
  );
  

• As the program grows, breaking the draw function into helpers, like drawMatrices, would be good.

• Consider making the application more friendly to various screens with dynamic sizes (not done in my rewrite below, but left as an exercise). You can see my version overflows the screen.

Suggested rewrite:

const matrices = [
 [
 [0, 1, 2],
 [1, 2, 3],
 [2, 3, 4],
 ],
 [
 [1, 2, 3],
 [2, 3, 4],
 [3, 4, 5],
 ],
 [
 [2, 3, 4],
 [3, 4, 5],
 [4, 5, 6],
 ],
];
function setup() {
 createCanvas(windowWidth, windowHeight);
}
function draw() {
 background("black");
 fill("white");
 textSize(20);
 let sum = 0;
 for (const [matIndex, matrix] of matrices.entries()) {
 for (const [rowIndex, row] of matrix.entries()) {
 for (const [colIndex, value] of row.entries()) {
 sum += value;
 const x = 500 + 20 * colIndex;
 const y = 100 + 120 * matIndex + 30 * rowIndex;
 text(value, x, y);
 }
 }
 }
 textSize(30);
 text("Sum=", 500, 500);
 text(sum, 500, 540);
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.11.0/p5.js"></script>

• Will.Octagon.Gibson: Thanks for your comprehensive review. Now I have a better understanding of the difference between "x=1;" and "let x=1;". For an example of my code that attempts to work well at different screen sizes, check out my Change Change game.