Köttur
Köttur

Reputation: 19

OR (||) operator in angular.forEach not working?

I have the following json array, and need to remove some of them by certain conditions:

var data = [
             {data1: "aaa", data2: "bbb", data3: "ccc"},  // First
             {data1: "ddd", data2: "eee", data3: "fff"},  // Second
             {data1: "ggg", data2: "hhh", data3: "iii"},  // Third
             {data1: "jjj", data2: "eee", data3: "lll"}   // Fourth
           ];
angular.forEach(data, (item, i) => {
  if (item.data2 == 'eee' || item.data3 == 'iii') {
    data.splice(i, 1);
  }
});
console.log(data);

In the above case, I need to remove second (data2 has "eee"), third (data3 has "iii"), and fourth (data2 is "eee") object from the data array. However, the third object is not spliced and remains in data array.

Could I use OR (||) operator in this manner? If not, what is the proper way to remove elements from array using multiple conditions?

I've been scratching my head around for hours with this problem, but perhaps I'm missing something.

Upvotes: 1

Views: 260

Answers (4)

Jose Hermosilla Rodrigo
Jose Hermosilla Rodrigo

Reputation: 3683

As I commented your code is not working because when looping throught the array, if you remove an item, the next item will place in the removed item index, index increases in one, and this item will be never evaluated.

var data = [
  {data1: "aaa", data2: "bbb", data3: "ccc"},  // First
  {data1: "ddd", data2: "eee", data3: "fff"},  // Second
  {data1: "ggg", data2: "hhh", data3: "iii"},  // Third
  {data1: "jjj", data2: "eee", data3: "lll"}   // Fourth
];

// Won't work
for(var i = 0; i < data.length; i++){
  var item = data [i];
  if (item.data2 == 'eee' || item.data3 == 'iii') {
    data.splice(i, 1);
  }
}
console.log(data);

One solution is to loop in reverse mode. In that case removing a item will not affect the next item.

var data = [
  {data1: "aaa", data2: "bbb", data3: "ccc"},  // First
  {data1: "ddd", data2: "eee", data3: "fff"},  // Second
  {data1: "ggg", data2: "hhh", data3: "iii"},  // Third
  {data1: "jjj", data2: "eee", data3: "lll"}   // Fourth
];

for(var i = data.length -1; i >= 0; i--)
  if (data[i].data2 == 'eee' || data[i].data3 == 'iii')
    data.splice(i, 1);

console.log(data);

If you want to keep a forEach loop style you can do your own forEachReverse :

Array.prototype.forEachReverse = function (callback){ 
  var arr = this;
  for(var i = arr.length -1; i >= 0; i--) 
  	callback(arr[i], i, arr);
};


var data = [
  {data1: "aaa", data2: "bbb", data3: "ccc"},  // First
  {data1: "ddd", data2: "eee", data3: "fff"},  // Second
  {data1: "ggg", data2: "hhh", data3: "iii"},  // Third
  {data1: "jjj", data2: "eee", data3: "lll"}   // Fourth
];

data.forEachReverse((item, i)=>{
  if (item.data2 == 'eee' || item.data3 == 'iii') {
    data.splice(i, 1);
  }
});

console.log(data);

As other answer suggested filter is cleaner and will filter your list with a condition.

Upvotes: 0

8protons
8protons

Reputation: 3959

When checking the output in console, you'll see that what is being returned is an array:

[Object, Object]

which containts

[{'aaa', 'bbb', 'ccc'}, {'ggg', 'hhh', 'iii'}]

Which tells us that your code is deleting the 2nd and 4th array objects. As proof, comment out your foreach routine to find that the integrity of the original array is preserved.

Why? Well, you're logical OR ( || ) operator is working as expected!

angular.forEach(data, (item, i) => {
  if (item.data2 == 'eee' || item.data3 == 'iii') {
    data.splice(i, 1);
  }
});

When the object item data2 == 'eee' or data3 == 'iii' in the current object being inspected, your code says to splice (in other words, remove) the elements starting at i and ending at i in the data array. This is expressed by the fact that you wrote splice(i, 1); had you written something like splice(i, 3), for example, you'd remove the object elements starting at i and ending at i+2. Another way to look at is by saying, "splice(i, n) means to remove n elements starting at index i".

So each time your comparison expression is evaluated, you're commanding that the current element i and only 1 element from i (which is i itself) be removed from the data array; that current element happens to be the objects that contain eee and iii. Run through your loop if you must, counting i to prove it to yourself that data[1] and data[3] are being removed.

If you're asking how to remove the specific items of the objects when item data2 == eee or item data3 == iii, consider the following:

var data = [
             {data1: "aaa", data2: "bbb", data3: "ccc"},  // First
             {data1: "ddd", data2: "eee", data3: "fff"},  // Second
             {data1: "ggg", data2: "hhh", data3: "iii"},  // Third
             {data1: "jjj", data2: "eee", data3: "lll"}   // Fourt
           ];
angular.forEach(data, (item, i) => {
  if (item.data2 == 'eee') {
    delete data[i].data2;   
  } 
  else if (item.data3 == 'iii') {
    delete data[i].data3;
  }
});
console.log(data);

Which returns:

[{'aaa', 'bbb', 'ccc'}, {'ddd', 'fff'}, {'ggg', 'hhh'}, {'jjj, 'lll'}]

A working demonstration is at this JSFiddle. Though your OR ( || ) expression does indeed "work", it's not exactly in the way you intended simply because splice is being used to remove an object instead of an item. Thus, we have to change our logic a bit.

Upvotes: 0

j3ff
j3ff

Reputation: 6089

Use filter instead of splice.

splice relates on the index of the item therefore when you remove item in the loop the index number become invalid.

var data = [
             {data1: "aaa", data2: "bbb", data3: "ccc"},  // First
             {data1: "ddd", data2: "eee", data3: "fff"},  // Second
             {data1: "ggg", data2: "hhh", data3: "iii"},  // Third
             {data1: "jjj", data2: "eee", data3: "lll"}   // Fourth
           ];

function keepItemCondition(item) {
  return !(item.data2 == 'eee' || item.data3 == 'iii');
}

var filtered = data.filter(keepItemCondition);

console.log(filtered);
<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.23/angular.min.js"></script>

Upvotes: 1

Johan Kvint
Johan Kvint

Reputation: 917

Use filter instead, When using splice you change the array you want to iterate on which is a no no:

var data = [
             {data1: "aaa", data2: "bbb", data3: "ccc"},  // First
             {data1: "ddd", data2: "eee", data3: "fff"},  // Second
             {data1: "ggg", data2: "hhh", data3: "iii"},  // Third
             {data1: "jjj", data2: "eee", data3: "lll"}   // Fourth
           ];
var filtered = data.filter(function (element) {
    return element.data2 !== 'eee' && element.data3 !== 'iii';
});
console.log(filtered);

Upvotes: 0

Related Questions