Variables declared inside a loop are preserving the prior values

Question

I've been trying to solve this on my own for hours and am raising my hand now to see if anyone has any ideas.

The Problem

I'm making a dynamic sidebar filter that iterates over a groups prop and returns a Link with a query prop to update the router. There's a checkbox displayed and is flagged based on if that group id is currently excluded.

The Code

render: function() {
  return (
    <ul>
    {this.props.query.filter == 'connections' ?
     // the array of groups to build the filter for
     this.props.groups.map(function (group) {
     // this array would start with the currently excluded ids, and end with the new list of ids
     var new_ids = this.context.router.getCurrentQuery().exclude || [];
     var index = new_ids.indexOf(group.id.toString());
     console.log('my id: ' + group.id);
     console.log('starting: ' + new_ids);
     // again, the array is excluded ids, so if it's not in the array it should be checked
     var selected = index == -1;
     if (selected) {
       // if this link is clicked, add the id to the excludes
       new_ids.push(group.id);
     }
     else {
       // if this linked is clicked, remove the id from the excludes
       new_ids.splice(index, 1);
     }
     console.log('ending: '+new_ids);
     // return the preloaded link
     return <Link  key={group.name}
                   to="feed"
                   query={{filter: 'connections', exclude: new_ids}}>
              <small>{group.name}</small>
            </Link>;
      }, this) : null }
   </ul>
);

This worked when I was using input type="checkbox" and an event handler with this.transitionTo, but I wanted to use the Link components to handle the requests instead of an event handler.

The Results

The page works fine for the first click (query.exclude == undefined) but after that the new_ids is mutating with each iteration. This is the console output...

id: 11
starting: 
new ids: 11
id: 6
starting: 
new ids: 6
id: 21
starting: 
new ids: 21

After you click one (say the first group -- id 11, it messes up)...

id: 11
starting: 11
new ids: // this is correct, it removes 11 from the list
id: 6
starting: // this should be 11, instead its inheriting the prior value
new ids: 6 // this should be 11, 6
id: 21
starting: 6 // this should be 11, instead its inheriting the prior value
new ids: 6,21 // this should be 11, 21

I've tried making this iteration a for ... loop instead of the .map() but that didn't help. I've also moved the initial excluded_ids out of the iteration, but again same result.

Again, all this is supposed to do is generate the values of the query.exclude prop for the navigation based on the result of clicking the link.

Any ideas as to what could be up would be appreciated. Thank you.

What do you want to do with the mapped Array? `Array.prototype.map` returns an array yet you aren't doing anything with it. — MinusFour, Aug 26 '15 at 15:41
I updated the code... it's just building a list of Links for the render function. — BradByte, Aug 26 '15 at 15:45
I'm looking at your second result set, why would the second `starting:` tag get you `11` instead of empty? You are essentially grabbing the `new_ids` which holds the reference to the `exclude` array and then remove the `11` value. Hence, next iteration there's no `11`. — MinusFour, Aug 26 '15 at 16:01
The second set simulates what happens when you click one of the links... so the URL query.exclude contains id 11.. hence the starting value. The issue is that they all should start with 11 in the second set because in each iteration I call `var new_ids = this.context.router.getCurrentQuery().exclude || [];` so it should always be getting the current query params. — BradByte, Aug 26 '15 at 16:04
And it does get the current query parameters but you are mutating it with `Array.prototype.splice` when you delete it from the array. — MinusFour, Aug 26 '15 at 16:10
That would make sense, but my thought was by assigning it to `new_ids` I would not mutate the "source".. is there a way I can accomplish it? I know in Ruby you can execute those functions without the bang and it won't apply it.. — BradByte, Aug 26 '15 at 16:13
`new_ids` won't copy the array contents but the `exclude` array reference. In other words you aren't getting a copy of the array, you are getting the same array. — MinusFour, Aug 26 '15 at 16:14
Oh gotcha. Based on your info I found this question too.. http://stackoverflow.com/questions/7486085/copying-array-by-value-in-javascript. — BradByte, Aug 26 '15 at 16:16

MinusFour · Answer 1 · 2015-08-26T17:13:42.977

Here's how you can avoid mutating the exclude array.

render: function() {
  return (
    <ul>
    {this.props.query.filter == 'connections' ?
     // the array of groups to build the filter for
     this.props.groups.map(function (group) {
     // this array would start with the currently excluded ids, and end with the new list of ids
     var new_ids = this.context.router.getCurrentQuery().exclude || [];
     console.log('my id: ' + group.id);
     console.log('starting: ' + new_ids);
     
     var found = false;
     new_ids = new_ids.filter(function(exclude){
         if(exclude != group.id.toString()){
            return true;
         } else {
            found = true;
            return false;
         }
     });

     if(!found) {
        new_ids.push(group.id);
     } 
     //new_ids now has a copy of the exclude array with the values excluded.

     console.log('ending: '+new_ids);
     // return the preloaded link
     return <Link  key={group.name}
                   to="feed"
                   query={{filter: 'connections', exclude: new_ids}}>
              <small>{group.name}</small>
            </Link>;
      }, this) : null }
   </ul>
);

Array.prototype.filter basically traverses the array and inspect each element. If the return value is true it adds to a new array else the element being iterated is discarded.

Here's a non-react script working in a similar way.

var group = [1,2,3,4,5,6,7,8,9,10,11];
var excluded = [11];
var results = document.getElementById('results');

group.forEach(function(group){
    var new_ids = excluded;
    results.innerHTML += 'my id: ' + group + '\n';
    results.innerHTML += 'starting: ' + new_ids + '\n';
  
    var found = false;
    new_ids = new_ids.filter(function(exclude){
       if(exclude != group){
           return true;
       } else {
           found = true;
           return false;
       }
    });
     
    if(!found){
        new_ids.push(group);
    }
    results.innerHTML += 'ending: ' + new_ids + '\n';
});

<pre id="results"></pre>

For some reason it this wasn't building the new array correctly when I am on Chrome.. but the copying of the array with `.slice()` does. — BradByte, Aug 26 '15 at 16:55
It works, but it seems to take a little longer to process than using the `.slice()`. You definitely pointed me in the right direction though! — BradByte, Aug 26 '15 at 17:17

score 0 · Accepted Answer · answered Aug 26 '15 at 17:10

My problem was due to not fully understanding the variable assignment to the array. I thought I was copying the array, but I was just making a reference to it. So, I was actually mutating the original value.

I fixed this by changing my variable new_ids to receive the results of Array.prototype.slice().

slice does not alter. It returns a shallow copy of elements from the original array. -MDN

So my new line looks like this:

var new_ids = (this.context.router.getCurrentQuery().exclude || []).slice();

Variables declared inside a loop are preserving the prior values

2 Answers2