Hayatomo Hayatomo - 5 months ago 9
Javascript Question

What is wrong with this use of "forEach"?

I expect the

errorLogArrayList.length
to be 3, but the result is 2.

Could anyone please tell me why this code doesn't work as I expect and how to fix it?

CODE:

var logArrayList = [
[1,2,3],
[1,2,"error"],
[1,2,"error"],
[1,2,"error"]
]

var errorLogArrayList = [];

console.log(logArrayList);
console.log("======================");


logArrayList.forEach(function(logArray, index, self) {

if(logArray[2] === "error") {

var errorArray = self.splice(index, 1)[0];
errorLogArrayList.push(errorArray);

}

});

// I expect this value to be 3, but the result is 2.
console.log("errorLogArrayList.length is " + errorLogArrayList.length)

console.log(errorLogArrayList);

console.log("======================");

// I expect this value to be 1, but the result is 2.
console.log("logArrayList.length is " + logArrayList.length);
console.log(logArrayList);


LOG:

[ [ 1, 2, 3 ],
[ 1, 2, 'error' ],
[ 1, 2, 'error' ],
[ 1, 2, 'error' ] ]
======================
// I expect this value to be 3, but the result is 2.
errorLogArrayList.length is 2
[ [ 1, 2, 'error' ], [ 1, 2, 'error' ] ]
======================
//I expect this value to be 1, but the result is 2.
logArrayList.length is 2
[ [ 1, 2, 3 ], [ 1, 2, 'error' ] ]

Answer

You can just do a filter operation:

var errorLogArrayList = logArrayList.filter(function(array) {
  return array[2] === 'error';
});

logArrayList = logArrayList.filter(function(array) {
  return array[2] !== 'error';
});

Unfortunately this requires some duplicate iterations. You can use _.partition from lodash if you want (looks a little cleaner):

var lists = _.partition(logArrayList, function(array) {
  return array[2] === 'error';
});

var errorLogArrayList = lists[0];

logArrayList = lists[1];

The problem with your current approach is that you are trying to keep track of multiple states and modify two arrays at the same time, and this is leading to some data conflicts. filter is usually a little more declarative, and it doesn't mutate the original array, but instead returns a new one.

Edit

Wanted to add a method with reduce as suggested by @zerkms in the comments:

var lists = logArrayList.reduce(function(agg, curr) {
  if(curr[2] === 'error') {
    agg[0] = (agg[0] || []).concat([curr]);
  } else {
    agg[1] = (agg[1] || []).concat([curr]);
  }

  return agg;
}, []);

That does the same as the partition example.

Comments