angularjs forEach and splice

Question

I have an array like this:

$scope.emails = [
  {"key":"Work","value":"user@domine.com"},
  {"key":"","value":""},
   {"key":"Work","value":"user2@domine.com"}
  {"key":"","value":""}];

So, I want to remove empty emails but angular forEach method removing only one object that is last object why???.

js code

angular.forEach($scope.emails, function(email, index){
     if(email.value ===""){
       $scope.emails.splice(index, 1);

     } 

    });

where I am doing wrong

JS Bin

Please include the relevant code in your question instead of just linking to JSBin. — James Allardice, Jun 13 '14 at 10:19

score 59 · Accepted Answer · answered Jun 13 '14 at 10:19

59

The problem is that you remove elements from the array during the loop so later items are at different indices. You need to loop backwards instead:

for (var i = $scope.emails.length - 1; i >= 0; i--) {
    if (!$scope.emails[i].value) {
        $scope.emails.splice(i, 1);
    }
}

Here's an updated example.

answered Jun 13 '14 at 10:19

James Allardice

164,175
21
332
312

The second argument to `splice` indicates how many elements have to be removed, so if you can determine it before, you can do it also in one single `splice`. For example, you can remove the last two emails of your array with `$scope.emails.splice(1,2)` (or `$scope.emails.splice(1)`, which will remove everything after the first element). – link Jun 13 '14 at 10:22
@James Allardice Thanks for your replay. It's working fine. But my question is can't we do this using forEach – chandu Jun 13 '14 at 10:23
@chandu - No, `forEach` iterates over the array in the wrong direction so you'll have to use a normal loop. – James Allardice Jun 13 '14 at 10:24
@link - That's only going to work when the elements in question are adjacent to each other in the array. It's fine in this case but it isn't very robust. – James Allardice Jun 13 '14 at 10:24
1

@JamesAllardice of course, that's why I added "if you can determine it before" :) OP's question is not so clear, so I thought to add it as a remark in case his situation allows it. – link Jun 13 '14 at 10:28

score 3 · Answer 2 · answered Jan 15 '17 at 01:01

Like others have pointed out, the culprit of the code is the array got removed. To get around with angular.forEach, you can try the additive/assignment approach:

var filteredEmails = [];
angular.forEach($scope.emails, function(email, index){
    if(email.value !==""){
        filteredEmails.push(email);
    }
});

$scope.emails = filteredEmails;

score 2 · Answer 3 · edited Sep 19 '17 at 18:41

2

indexOf returns -1 when it doesn't find an item.

A way to remove an item, and to avoid removing the last one when not found, is:

var index = $scope.items.indexOf($scope.oldItem);

if (index != -1) {
  $scope.items.splice(index, 1);
}

edited Sep 19 '17 at 18:41

jacefarm

6,747
6
36
46

answered Sep 19 '17 at 16:45

Muhammad Azam

535
7
12

Tek · Answer 4 · 2014-06-13T11:24:57.130

1

describe('Foreach Splice', function () {
  it('splicing', function () {

    var elements = [
      {name: "Kelly", age: 16},
      {name: "", age: 17},
      {name: "Becky", age: 18},
      {name: "", age: 18},
      {name: "Sarah", age: 19},
      {name: "", age: 20},
      {name: "", age: 22},
      {name: "Mareck", age: 21},
      {name: "", age: 21},
      {name: "Mareck", age: 21}
    ];

    removeEmptyEntry(elements);
    console.log(elements);
  });


  function removeEmptyEntry(elements) {
    elements.forEach(function (element, index) {
      if (!element.name) {
        elements.splice(index, 1);
        removeEmptyEntry(elements);
      }
    });
  }
});

edited Jun 13 '14 at 11:24

answered Jun 13 '14 at 11:02

Tek

1,178
1
12
22

Jut I want to remove empty values not 2nd index element – chandu Jun 13 '14 at 11:03
Ah what you mean is that you want to remove empty elements ? – Tek Jun 13 '14 at 11:04

score 0 · Answer 5 · answered Mar 16 '18 at 14:54

I haven't tried this with AngularJs, but with Angular 4 a similar way of this works pretty well.

angular.forEach($scope.emails, function(email){
 if(email.value ===""){
   $scope.emails.splice($scope.emails.indexOf(email), 1);
 } 

});

Angular 4 version:

this.emailArray.forEach(email => {
  if (email.value == "") {
    this.emailArray.splice(this.emailArray.indexOf(email),1);
  }
});

angularjs forEach and splice

5 Answers5

Linked