bizzare undefined variable

Question

I have a multi-dimensional array containing objects, and I wish to remove objects from the array if they contain a specific property.

COFFEESCRIPT

for dataColumn in allDataColumns
    for brentSpiner, i in dataColumn
        console.log i, brentSpiner.refreshRate
        #dataColumn.remove i if brentSpiner.refreshRate is -1

the above console.log works as expected when the line bellow is commented out

for dataColumn in allDataColumns
    for brentSpiner, i in dataColumn
        console.log i, brentSpiner.refreshRate
        dataColumn.remove i if brentSpiner.refreshRate is -1

the above errors out like so: brentSpiner is undefined console.log(i, brentSpiner.refreshRate); in firebug

how on earth could the presence of the second line cause the variable in the above line to become undefined?

RENDERED JAVASCRIPT

Fails

for (_i = 0, _len = allDataColumns.length; _i < _len; _i++) {
  dataColumn = allDataColumns[_i];
  for (i = _j = 0, _len1 = dataColumn.length; _j < _len1; i = ++_j) {
    brentSpiner = dataColumn[i];
    console.log(i, brentSpiner.refreshRate);
    if (brentSpiner.refreshRate === -1) {
      dataColumn.remove(i);
    }
  }
}

Works

for (_i = 0, _len = allDataColumns.length; _i < _len; _i++) {
  dataColumn = allDataColumns[_i];
  for (i = _j = 0, _len1 = dataColumn.length; _j < _len1; i = ++_j) {
    brentSpiner = dataColumn[i];
    console.log(i, brentSpiner.refreshRate);
  }
}

(side note: .remove has been added to the Array prototype via Resig)

UPDATE

This was a logic error on my part. View the approved answer to see why. Bellow is what I ended up doing and worked well:

for dataColumn in allDataColumns
    i = 0
    len = dataColumn.length
    while i < len
        if dataColumn[i].refreshRate is -1
            dataColumn.remove i
            len--
        i++

Rendered

for (_i = 0, _len = allDataColumns.length; _i < _len; _i++) {
  dataColumn = allDataColumns[_i];
  i = 0;
  len = dataColumn.length;
  while (i < len) {
    if (dataColumn[i].refreshRate === -1) {
      dataColumn.remove(i);
      len--;
    }
    i++;
  }
}

Pulling elements off an array that is being looped over is an unstable operation. By fixing the loop length, coffeescript ensures that this will run over the end, rather than skipping elements, which may be harder to catch. — Aaron Dufour, Jul 12 '12 at 05:09
Do you think my `while` loop version contains risks as well? — Fresheyeball, Jul 12 '12 at 20:07
Yes, because when you remove an element, you skip over the next element. Example: list is `[1,2,3]`, `i=0`. If we decide to remove `i`, then at the end of the while loop we have `[2,3]`, `i=1`. The next item we'll look at is `3`, so we've skipped `2`. I would decrement `i` inside the `if` that removes an element, or put the increment in an `else` block. I would also suggest not freezing the length of the list - having to track the length through removes feels weird, and getting the length from the array is constant time anyway. — Aaron Dufour, Jul 13 '12 at 14:38
@AaronDufour You have a point. I have added a decrement to the if statement so in instances where the array lost one in length the index stays the same for the next iteration. Would you care to write an answer for the way you feel it should be done? — Fresheyeball, Jul 13 '12 at 15:16
Done. Its probably pretty much the same as your code, with an extra line at the beginning of the loop to allow us to more easily see how it correlates to the `for..in` version. — Aaron Dufour, Jul 13 '12 at 15:34
dear downvoter, care to comment on why this is not a valid question? — Fresheyeball, Jul 24 '12 at 19:40

score 0 · Accepted Answer · answered Jul 12 '12 at 00:54

0

Coffeescript is "freezing" the length of the list in _len1. When you remove an entry, you'll run off the end of the list in the inner loop.

Looks like a bug to me. (Unless there's some language feature by which you're supposed to inform Coffeescript that the array length may change.)

answered Jul 12 '12 at 00:54

Pointy

405,095
59
585
614

http://tinyurl.com/d47ul7m trippy. That makes sense, totally my bug. Do you have any ideas on how to get around this? – Fresheyeball Jul 12 '12 at 00:57
No; I'm not a Coffeescript person :-) If I were, I'd definitely report this as a bug if I could find no mention of such a situation in the language documentation. – Pointy Jul 12 '12 at 00:58
@Fresheyeball Loop through the array backwards. I'm not sure of the syntax for this in CoffeeScript. – Russ Ferri Jul 12 '12 at 01:12
@RussFerri why would that help? – Fresheyeball Jul 12 '12 at 01:24
@Fresheyeball did you see my answer? Btw: If you iterate backwards, it would not matter that the length of the array would shrink, as you delete from the back and walk to the front. – Kijewski Jul 12 '12 at 01:45
@Kay I did see your answer. Backwards makes sense then, I would still worry about falling off, or worse re-encountering the same item. – Fresheyeball Jul 12 '12 at 01:47
@Fresheyeball if you are currently at item i, the next item you would iterate over would be `i-1`, regardless whether i was deleted or not. You cannot "fall off" off the front, since the first element is always 0. – Kijewski Jul 12 '12 at 01:49
@RussFerri out of curiosity, how do you go backwards with a Coffeescript "for ... in" loop? – Pointy Jul 12 '12 at 01:49

score 0 · Answer 2 · answered Jul 13 '12 at 15:33

Adding/removing elements to/from a list that is being looped over is a dangerous operation, and it is very difficult for a language to deal with this automatically. If you need to do something like this, you should avoid for..in loops and stick with a while loop. Here is how I would write that code:

for dataColumn in allDataColumns
    i = 0
    while i < dataColumn.length
        brentSpiner = dataColumn[i]
        console.log i, brentSpiner.refreshRate
        if brentSpiner.refreshRate is -1
            dataColumn.remove i 
            i--
        i++

I'll also transform your code in steps so we can see how I got there:

for dataColumn in allDataColumns
    for brentSpiner, i in dataColumn
        console.log i, brentSpiner.refreshRate
        dataColumn.remove i if brentSpiner.refreshRate is -1

First, we need to make the inner loop into a while loop. We're going to loop as long as our counter variable is less than the length, and increment the counter at the end of the loop. To retain syntax, the first thing we do inside the loop is set the looping variable brentSpiner.

for dataColumn in allDataColumns
    i = 0
    while i < dataColumn.length
        brentSpiner = dataColumn[i]
        console.log i, brentSpiner.refreshRate
        dataColumn.remove i if brentSpiner.refreshRate is -1
        i++

Now, we have a problem if we remove an element from the list, because the next element will be skipped. Example: list is [1,2,3], i=0. If we decide to remove i, then at the end of the while loop we have [2,3], i=1. The next item we'll look at is 3, so we've skipped 2. To fix this, we decrement the counter each time we remove an item that is at or before the current counter.

for dataColumn in allDataColumns
    i = 0
    while i < dataColumn.length
        brentSpiner = dataColumn[i]
        console.log i, brentSpiner.refreshRate
        if brentSpiner.refreshRate is -1
            dataColumn.remove i 
            i--
        i++

If we were also adding elements, we would have to be very careful. We might have to increment the counter, depending on where the element is being inserted and whether it should be handled in the loop.

bizzare undefined variable

2 Answers2