for loop index variable issues (noob)

Community Help
1

Hi there,

I’m trying to run a class function that depends on the incrementing variable in a for loop, but it’s evaluating to the final value of the incrementing variable for every loop. I’m probably mistaking how for loops are interpreted in Observable, but I’m not sure what I’m missing. Here is the notebook: https://observablehq.com/d/6219671b09a278d6
and the relevant cell is

{ 
  for (let i = 0; i < testRows.length; i++) {
    testRows[i].changeSaturation(2, 5 * i);
  }
}

is it because I’m mutating the Row objects?

2

Hi Solar,

Mind clicking ‘share link’ so that people can check out the referenced notebook? Right now it’s private.

image
image.png710×230 8.4 KB

-Tom

3

Done

4

This one was a bit of a puzzler! Here’s a suggestion with a tweak: https://observablehq.com/compare/6219671b09a278d6...298f3756112e13ee

The code

testRows = makeRows(5, 20, randomHsla(4))

Was making all the rows with the same color - whatever randomHsla(4) came up with. This meant that we were working with precisely the same array, so any modifications to that array would modify it everywhere it was referenced. This instead calls randomHsla for each row, so the mutating the color in one place doesn’t change it everywhere else.

5

Yah, I noticed that just a few minutes ago! Thanks for the tip!

6

I wanted to keep the same color per row so I ended up changing the makeRows function slightly. The problem was the nested arrays

makeRows = function(numRows, colLength, hsla, randomColor = null) {
  let rows = [];
  for (let i = 0; i < numRows; i++) {
    let newhsla = hsla.map((val) => {return val.slice()});
    rows.push(new Row({values: [0,0,0,0], colLength, hsla: newhsla, x: 0, y: i*10, randomColor}));
  }
  return rows;
}
7

With regards to the issue that @tom pointed out, let me also suggest that you edit the constructor in the Row class so that it makes new copies of the input arrays, as otherwise you may end up with more hard-to-track-down dependencies in the future. So I would change these lines:

    this.values = values;
    this.hsla = hsla;

to

    // copy array
    this.values = values.slice();
    // copy once-nested array:
    this.hsla = hsla.map(a => a.slice());

This way each Row object gets its own copy of these arrays and running methods on one row object won’t affect the arrays used by other Rows.

Next, I noticed that the evaluation order of some of your cells may not be what you want it to be. Let me be explicit. First, this cell runs:

testRows = makeRows(5, 20, randomHsla(4))

Then, both this cell (the “canvas” cell):

{
  const ctx = DOM.context2d(350, 100);
  drawRows(testRows, ctx);
  return ctx.canvas;
}

and this cell (the “for-loop” cell):

{ 
  for (let i = 0; i < testRows.length; i++) {
    testRows[i].changeSaturation(2, 5 * i);
  }
}

become able to run. However, the execution order is not guaranteed, and in my tests it looks like the for-loop cell tends to run before the canvas cell when I first load the notebook, but the canvas cell runs first if I re-evaluate the testRows cell.

In the latter case, after the automatic evaluation finishes, the canvas cell will still show the initial state of testRows and not what testRows is after the for-loop cell mutates it, and this might be misleading. Of course, if I then manually evaluate the canvas cell, then I see that the third row of colors has become desaturated, as now it’s drawing the updated testRows.

If you want the canvas cell to always run after the for-loop, you can name the for-loop cell something:

forLoop = { 
  for (let i = 0; i < testRows.length; i++) {
    testRows[i].changeSaturation(2, 5 * i);
  }
}

and then make the canvas cell depend on this as follows:

{
  forLoop;
  const ctx = DOM.context2d(350, 100);
  drawRows(testRows, ctx);
  return ctx.canvas;
}
8

Woot! Thanks for all of the advice and info, I’ll definitely use it.

2 Likes
9

Changing the constructor to make clones of the hsla array means I don’t have to also clone it in the makeRows function, correct?

10

Yes, that’s right.