Calling a function inside a loop breaks it

Question

I have a function that loops an object from a MongoDB collection. It's all possible connections for some mail transportation posts. Once I get one connection, I want to immediately remove the inverse connection from the connections object, for example, postA=1 and postB=2, I want to remove postA=2 and postB=1 (removeConnection function does that).

I can't understand why it only returns one 'A' on the console when I try to run that function inside calculateRoute, and returns three 'A' (which is what it should) when I remove it. That function is somehow breaking the loop.

calculatedRoutes = calculateRoute(store.postid, client.postid, connections, []);

function calculateRoute(actualPost, finalPost, connections, routes) {
    for(i=0; i < connections.length; i++) {
        if(actualPost == connections[i].postA) {

            console.log('A');

            // If I remove this, the console shows A three times. If I keep this, only shows 1 time.
            connections = removeConnection(connections[i].postB, connections[i].postA, connections);
        }
    }

    return routes;
}

function removeConnection(postA, postB, connections) {
    for(i=0; i < connections.length; i++) {
        if(connections[i].postA == postA && connections[i].postB == postB) {
            delete connections[i];
            //break;
        }
    }

    return connections;
}

Your function changes the collection you are iterating over. — PM 77-1, Nov 21 '15 at 23:33
Yes, that's the idea but it does not affect the iteration. I even tried changing `connections` to `newConnections` with the return of the function but the same happens. — mignz, Nov 21 '15 at 23:37
I agree with PM, but I think that can be fixed if you increment `i` only if you don't `removeConnection()`. Maybe. — Alderin, Nov 21 '15 at 23:38
I tried not iterating over (renamed and then removed the attribution), still breaks the loop. — mignz, Nov 22 '15 at 00:10

score 1 · Answer 1 · answered Nov 21 '15 at 23:38

1

It appears that you are modifying the collection that you are iterating over when you callremoveConnection. I would venture to say that after the first loop, connections.length is less than your loop control variable, which would cause the loop to terminate. What are the contents of connections after the function call?

In general, directly modifying a collection you're iterating over is bad practice. A better option would be to project the collection into a new one that contains the values you want (using map,filter,etc). That way your not mutating anything.

answered Nov 21 '15 at 23:38

Shawn Hartsell

51
4

you can also iterate over the collection in reverse order – Nick Nov 21 '15 at 23:45
@Nick There's many different Swift could solve the problem. The main bug in the current implementation though is that the collection is being directly modified by the "delete" statement in removeConnections – Shawn Hartsell Nov 21 '15 at 23:49
Hmm it appears using `delete` doesn't alter the length of the array http://stackoverflow.com/a/5767335/1076471 – Nick Nov 21 '15 at 23:54
However, deleting an item with `delete` will cause it to return `undefined` so `if(actualPost == connections[i].postA)` would really be `if(actualPost == undefined.postA)` which is an error since undefined can't have a property. I'd recommend checking your console output – Nick Nov 21 '15 at 23:59
@Nick Doh! Completey forgot about that! Again, of of the many reason I avoid the delete operator hehe – Shawn Hartsell Nov 22 '15 at 00:01
I understand all that, but as I mention in a comment from my question, I changed `connections = removeConnection(conn...` to `newConnections = removeConnection(conn...` and it still breaks the loop and the `connections` object isn't being modified. – mignz Nov 22 '15 at 00:07

score 0 · Accepted Answer · answered Nov 22 '15 at 04:09

0

Fixed it by adding var to i=0 on each for. Can someone explain?

for(var i=0; i < connections.length; i++) {...

answered Nov 22 '15 at 04:09

mignz

1,779
2
14
19

1

oh that's because without declaring `i`, it is assumed to be a global variable and so `removeConnection` would essentially be setting `window.i = 0` and incrementing it to `i = connections.length`. That would cause it to behave just like a break – Nick Nov 22 '15 at 20:25

Calling a function inside a loop breaks it

2 Answers2