Reputation: 523
Using the following code, I'm supposed to check if the arrays (ai, jq, and rz) has users with the same 'interest' and if so, remove that user from all arrays. But it seems that the .splice() method is not be working as I log the arrays and they still contain that user. Any ideas?
Code:
function joinQueue(sid, interests, fn) {
var exists = false;
interests.forEach(function(interest) {
interest = interest.toLowerCase();
getTable(interest.charAt(0), function(table) {
table.forEach(function(data) {
if(data.interest == interest && !exists) {
var aa = 0;
ai.forEach(function(a) {
if(a.sid == data.sid) { ai.splice(aa, 1); console.log(ai);}
aa++;
});
var jj = 0;
jq.forEach(function(j) {
if(j.sid == data.sid) { jq.splice(jj, 1); console.log(jq); }
jj++;
});
var rr = 0;
rz.forEach(function(r) {
if(r.sid == data.sid) { rz.splice(rr, 1); console.log(rz); }
rr++;
});
fn(data);
exists = true;
}
});
});
});
if(!exists) {
interests.forEach(function(interest) {
interest = interest.toLowerCase();
getTable(interest.charAt(0), function(table) {
table.push({'sid': sid, 'interest': interest});
});
});
fn();
}
}
Upvotes: 0
Views: 112
Reputation: 2779
[Untested]. It is difficult to establish whether it makes any difference without access to testable code (input data and some other variables are missing).
It is best to avoid to add or remove items to an array as you are looping over it. Best is to loop over a clone of the array rather than the array itself. Try with the code below to see if it makes any difference.
function joinQueue(sid, interests, fn) {
var exists = false;
interests.forEach(function(interest) {
interest = interest.toLowerCase();
getTable(interest.charAt(0), function(table) {
table.forEach(function(data) {
if(data.interest == interest && !exists) {
var sid = data.sid;
var sliceItOff = function(arr) {
arr.slice(0).forEach(function(d, i) {
// [warning]. if more than one item meets the condition (d.sid == sid),
// then you are likely to remove an item at the wrong index.
// A better approach is to build a new array rather than modify the old one
if(d.sid == sid) { arr.splice(i, 1); console.log(arr); }
});
};
sliceItOff(ai);
sliceItOff(jq);
sliceItOff(rz);
fn(data);
exists = true;
}
});
});
});
if(!exists) {
interests.forEach(function(interest) {
interest = interest.toLowerCase();
getTable(interest.charAt(0), function(table) {
table.push({'sid': sid, 'interest': interest});
});
});
fn();
}
}
Upvotes: 0
Reputation: 707436
Here's a bit safer version that uses a backwards for
loop traversal to avoid issues when the current item is removed.
function joinQueue(sid, interests, fn) {
var exists = false;
interests.forEach(function(interest) {
interest = interest.toLowerCase();
getTable(interest.charAt(0), function(table) {
table.forEach(function(data) {
if(data.interest == interest && !exists) {
var sid = data.sid;
var sliceItOff = function(arr) {
for (var i = arr.length - 1; i >= 0; i--) {
if(arr[i].sid == sid) {
arr.splice(i, 1);
}
}
};
sliceItOff(ai);
sliceItOff(jq);
sliceItOff(rz);
fn(data);
exists = true;
}
});
});
});
if(!exists) {
interests.forEach(function(interest) {
interest = interest.toLowerCase();
getTable(interest.charAt(0), function(table) {
table.push({'sid': sid, 'interest': interest});
});
});
fn();
}
}
Upvotes: 1