Javascript: forEach() loop to populate an array - closure issue

Question

Let's say we have an array of objects like:

var fruits = [ {name:"banana", weight:150},{name:"apple", weight:95},{name:"orange", weight:160},{name:"kiwi", weight:80} ];

I want to populate a "heavy_fruits" array with items from the "fruits" array above which weight is > 100. Here is my code:

var heavy_fruits = [];
myfruit = {};

fruits.forEach(function(item,index) {
  if ( item.weight > 100 ) { 
    myfruit ["name"] = item.name;
    myfruit ["weight"] = item.weight;
  }

heavy_fruits.push(myfruit);
});

However it shows: name:"orange", weight:160 name:"orange", weight:160 name:"orange", weight:160 name:"orange", weight:160

I know this is an issue with mixing closures and loops... but I read an article (http://zsoltfabok.com/blog/2012/08/javascript-foreach/) explaining that I would avoid this kind of issue using a forEach loop instead of the classic for loop.

I know I can use array methods like filter(), etc. but I'm asking that on purpose since I'm actually having troubles with a much bigger function that I cannot expose here... So I tried to summarize and simplify my issue description with "fruits".

Footton

myfruit is referencing to same object. Move myfruit = {}; in the forEach callback. And I'll suggest to use filter as var heavy_fruits = fruits.filter(f => f.weight > 100);. – Redhanded

It's not closure issue, it's about referencing same object. – Redhanded

Basically what you are doing is mutating object's properties and because the array is in fact storing references to the same object, when you change one of the object's property, the change is visible in each reference. – Fovea

Amongst all array methods forEach has got <= 2% use case. For this particular case you should use either filter or reduce. – Aciculum

I was having the same issue as the original poster, and the thing that fixed it for me was the first comment above by Tushar. The variable where the empty object is declared must be inside the forEach. – Puente

Answer 1

var heavy_fruits = [];
myfruit = {}; // here's your object

fruits.forEach(function(item,index) {
    if ( item.weight > 100 ) { 
        myfruit ["name"] = item.name;
        myfruit ["weight"] = item.weight; // you modify it's properties
    }

    heavy_fruits.push(myfruit); // you push it to the array
});

You end up with an array [myfruit, myfruit, myfruit, myfruit].

Now if you modify myfruit anywhere in the code, the change will be visible in every single occurence of myfruit. Why?

Because you are modifying the referenece to the object. In this example, your array stores just copies of your object. And if you change one of it, every single one will change, because they are all references.

To fix this with each iteration you should be creating a new object and then doing some stuff on it.

BTW, as a matter of fact, your if could just be like this:

if ( item.weight > 100 ) { 
    heavy_fruits.push(item); // if `item` only has `name` and `weight` properties
}

Fovea

Thank you so much to everyone who took time to help me to find the solution: Albzi, Tushar, Adrian and Ganga. However Adrian provided the solution + the clear explanation of my issue + why it does not work + how I can fix it. I've struggling with that issue for days! Regards – Footton

Answer 2

 fruits.forEach(function (item, index) {
  if (item.weight > 100) {
    myfruit = {};
    myfruit["name"] = item.name;
    myfruit["weight"] = item.weight;
    heavy_fruits.push(myfruit);
  }
}); 

Dottydoty

Please edit with more information. Code-only and "try this" answers are discouraged, because they contain no searchable content, and don't explain why someone should "try this". We make an effort here to be a resource for knowledge. – Redhanded

This is not different from the code in question. Except, this'll give array of two oranges than four. – Redhanded

heavy_fruits.push(myfruit); was out of the if scope ..that is the problem – Dottydoty

That is a code problem ..I don't find any other issues with that – Dottydoty

Answer 3

The shorter would use filter

var heavy_fruits = fruits.filter(x => x.weight > 100);

But if you realy want to use forEach do this way

var heavy_fruits = [];
fruits.forEach(x => {if(x.weight > 100) heavy_fruits.push(x)} );

Hurlow

Answer 4

forEach is not only an array method, so you might encounter it with DOM list of elements, that may have no map method.

As forEach is unfortunately not returning an array (as opposed to map), it's usage is pushing to an array you should predefine, as with the previous answers here.

var values = Array();
var subelem = document.querySelectorAll('#elemid subelem');
values.push(subelem.value);

Abbie