current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I would like to get a distinct array of objects. The code below is the first thing that came to mind.

Do you see any room for improvements?

function distinct(objectArray){

    var distinctResult = [];

    $.each(objectArray, function(i, currentObject){
        if(!exists(distinctResult, currentObject))
            distinctResult.push(currentObject);
    });

    return distinctResult;
}

function exists(arr, object){
    var compareToJson = JSON.stringify(object),
        result = false;
    $.each(arr, function(i, existingObject){
        if(JSON.stringify(existingObject) === compareToJson) {
            result = true;
            return false; // break
        }
    });

    return result;
}

var testList = [{ foo: 1 }, { foo: 1 }, { bar: 1 }, { bar: 2, baz: 3 }];

console.log(distinct(testList));

share|improve this question

1 Answer 1

up vote 2 down vote

Your code looks OK and will work but I have a few issues with it.

jQuery

You're using $.each in your distinct() function - this is fine, but it means I can now not use your function unless I support jQuery. jQuery is not really known as a general purpose utility library, so if I'm using Angular or Ember, I'm probably not going to want to use your function.

$.each is designed primarily for iterating the DOM rather than general arrays.

Reusability

Your distinct function is not very re-usable. You don't take a function as a parameter for matching, which means I have to rely on your method of determining equality. This might work for your problem space, but it does not for mine - Let us say I want to determine equality between base 64 strings. I cannot use your equality function there, and so if I have an array of base 64 strings I'm buggered.

Comparing JSON strings.

An interesting idea, I will give you that, but this is a bit... weird, and incurs a performance overhead. It certainly threw me off. While there is nothing intrinsically wrong with this approach, it feels dirty.

Behold lodash/uniq!

With all this said and done, there is already a utility function to perform this exact purpose in lodash (arguably the best general-purpose utility library in JavaScript). Granted, it only works on arrays, but so does your example. It also comes with the ability to specify your own matcher, and performs optimizations on sorted arrays.

var testList = [{ foo: 1 }, { foo: 1 }, { bar: 1 }, { bar: 2, baz: 3 }];
console.log(_.uniq(testList, function(obj) { return JSON.stringify(obj); });
share|improve this answer
    
Thanks. I disagree with "$.each is designed primarily for iterating the DOM rather than general arrays.". $(selector).each() is used for DOM element, while $.each() is a utility method which can be used instead of Array.prototype.forEach (pre IE9). As for comparing JSON strings, I agree that it feels a bit costly performance wise, but I couldn't think of another generic solution for comparing all properties on any level. Can you? :) –  filur Apr 9 at 10:50
    
@filur Regarding $.each, the documentation explicitly states it is designed for use with iterating the DOM. Regarding JSON strings, it feels clunky, and if I could, I would use some kind of unique identifier rather than a JSON string, but if it's the only way you can do it, then it's the only way you can do it :P –  A Red Herring Apr 9 at 10:52
    
You are linking to the wrong method: api.jquery.com/jquery.each Regarding the JSON string; I think that even if I started iterating all object properties and comparing them one by one, it would still be more costly in the end. –  filur Apr 9 at 10:54
1  
@filur I meant as in some kind of _id property that uniquely identifed each row, i.e, an id that represents a resource from a server. That would undoubtedly be faster than serializing an entire object to JSON and then comparing two strings. But obviously this doesn't work for arrays that aren't from a server –  A Red Herring Apr 9 at 11:01
1  
"there is nothing intrinsically wrong with this approach, it feels dirty." That's because the approach is wrong. Converting objects to JSON is not guaranteed to produce the same string every time on every browser, as the properties are not ordered and sorted. JSON-wise the content is the same, STRING-wise not guaranteed. –  Plantface Jul 31 at 7:53

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.