views:

52

answers:

1

I need to find which id numbers are missing inside s.data compared to users. Is there a better(smaller code) way to compare?

Thanks ;)

if(users.length != undefined)
{

    for(y=0;y<users.length;y++) 
    {
        var left = true;
        for(y2=0;y2<s.data.length;y2++) 
        {
            if(users[y].client_id==s.data[y2].client_id) {left = false;break;}
        }
        if(left) {users[y].ref.remove();delete users[y];}

    }

}
else if(!jQuery.isEmptyObject(users))
{
    var left = true;
    for(y2=0;y2<s.data.length;y2++) 
    {
        if(users.client_id==s.data[y2].client_id) {left = false;break;}
    }
    if(left) {users.ref.remove();users = {};}
}

Haven't checked if this is working code. :)

+3  A: 

First, off, the 2nd branch appears to be nothing but a specialization of the first branch. You can use this to either make the "2nd" users = [users] (in which case users really means users and not a-user) and eliminates the top branch entirely, or remove the the logic into a function invoked per-user.

Now, to tackle the inner loop: What this is a 'map' and a 'contains'. Looking at it just in terms of a contains:

// Returns true if any item in data.client_id (an array)
// is that of user.client_id
function dataContains (user, data) {
  for (var i = 0; i < data.length; i++) {
    if (data[i].client_id == user.client_id) {
      return true
    }
  }
  return false
}

Now the code is reduced to:

for (each user) {
  if (!dataContains(user, data)) {
    // do something here
  }
}

However, we could go one step further and use a generic 'contains' if we also have a 'map'. The final form is then:

var dataIds = map(data, function (x) { return x.client_id })
for (each user) {
  if (!contains(user.client_id, dataIds)) {
    ..
  }
}

Where the 'contains' is much more generalized:

// Returns true iff item is contained within arr
function contains (item, arr) {
  // Just do what the comment documentation says
}

If you are using jQuery you already have handy functions: 'contains' - inArray, and a "sorta" 'map' - map. However, be warned! The jQuery 'map' is really a flat-map and was given an incorrect name and incomplete documentation!

I believe ECMAScript ED5 has these functions standard.

Also, you could invert the client_id's in the data to object keys and simply test for key existence, which is O(1) vs. O(n) iff the look-up is built once (or at least much, much less than it's used) and so it may be "theoretically" better. The size of n makes a large difference if it will actually matter, if at all. In this case it's likely the look-up could be built incrementally and saved between times this code is executed.

var existingIds = {}
for (var i = 0; i < data.length; i++) {
  existingIds[data[i].client_id] = true
}
for (each user) {
  if (!existingIds[user.client_id]) {
    ..
  }
}
pst
Thanks mate! That's what i call an excellent answer!Too bad that i can add only 1+ :)
Beck