2
\$\begingroup\$

Use Case

So our lead programmer loves to follow the NIH Anti-Pattern and consequently I'm not allowed to use Underscore.js and can only use Vanilla JS... and we have to support IE8.

The goal is to produce an aggregation of unique values from nested collections buried within various collections of objects.

Desired Interface

The idea is to call a function to create an aggregation function give the list of the nested properties.

Example

The following would flatten all options collection that are buried under each object within the properties property.

var allUniqueOptions = aggregateByProperty('properties', 'options');
var allUniqueDoodadsOptions = aggregateByProperty('doodad', 'properties', 'options');

allUniqueOptions(myCollection);
allUniqueDoodadsOptions(myDoodadCollection);

Current Code

The code below works, but I'd like any advice on improving it for:

  • performance
  • readability
  • any further generalization

I have a jsFiddle created.

/* Returns the index of an object within a collection */
function arrayObjectIndexOf(arr, obj) {
    var search = JSON.stringify(obj);
    for ( var i = 0, k = arr.length; i < k; i++ ){
        if (JSON.stringify(arr[i]) == search) {
            return i;
        }
    };
    return -1;
};

/* Concatentates objects that don't exist in the source collection */
function concatIfNotExists(source, additions) {
    var result = source;
    for ( var i = 0, k = additions.length; i < k; i++ ) {
        var addition = additions[i];
        if (arrayObjectIndexOf(result, addition) === -1) {
            result.push(addition);
        }
    };
    return result;
};

/* Returns a nested collection buried within an object */
function getInnerCollection(obj, props) {
    var innerCollection = obj;

    for (var j = 0, l = props.length; j < l; j++) {
        if (innerCollection.hasOwnProperty(props[j])) {
            innerCollection = innerCollection[props[j]];
        } else {
            throw new TypeError("Property doesn't exist!");
        }
    }

    if (!innerCollection instanceof Array) {
        throw new TypeError("Inner property is not a collection!");
    }
    return innerCollection;
};

/* Returns an aggregation of objects based on collection properties */
function aggregateByProperty() {
    var props = Array.prototype.splice.call(arguments, 0);

    return function(objectCollection) {
        var result = [];
        for (var i = 0, k = objectCollection.length; i < k; i++) {
            var innerCollection = getInnerCollection(objectCollection[i], props);
            result = concatIfNotExists(result, innerCollection);
        }
        return result;
    }
};
\$\endgroup\$
  • \$\begingroup\$ Does the data always come in the format shown in the demo? \$\endgroup\$ – Joseph Apr 16 '14 at 22:12
  • \$\begingroup\$ No. I'm looking for a general case since I'd like to reuse this function. However, you can always expect the parameter passed into the result function to be a collection of objects. \$\endgroup\$ – Pete Apr 16 '14 at 22:16
3
\$\begingroup\$

From a once over :

  • This is a problem : var search = JSON.stringify(obj); because JSON.stringify() does not guarantee any order.. To be completely correct I think you are going to need to pass the properties to arrayObjectIndexOf that need to get compared ( that is, if uniqueness is determined by only comparing the provided properties, otherwise you are in a world of trouble )
  • "Inner property is not a collection!" <- Perhaps props[l-1] + " is not an array!", then you will provide which property is not an array, and be clear that you are expecting an array
  • "Property doesn't exist!" <- "Property " + props[j] + "does not exist" will give a better feedback as to which part is not working
  • JSHint.com could only find semicolon issues, which is good
\$\endgroup\$
  • \$\begingroup\$ I was hoping to save an extra loop using stringify, but I can see where this path will lead to trouble... Thanks for the advice on the error handling as well! I have to be less lazy on those messages. \$\endgroup\$ – Pete Apr 16 '14 at 22:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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