2
\$\begingroup\$

I have a deep equality comparison method that returns true when two objects are equal (see method Doc):

_.isEqual = function (obj1, obj2) {

  // Quick compare objects that don't have nested objects
  if (_.type(obj1) === _.type(obj2) && !_.isPlainObject(obj1) && !_.isArray(obj1)) {
    switch (_.type(obj1)) {
      case "function" :
        if (obj1.toString() !== obj2.toString()) return false;
        break;
      case "nan" :
        if (obj1 === obj2) return false;
        break;
      default:
        if (obj1 !== obj2) return false;
    }
  } else {

    // When target or comparison is falsy we compare them directly
    if (_.isFalsy(obj1) || _.isFalsy(obj2)) {
      if (obj1 !== obj2) return false;
    }
    for (var o in obj1) {
      switch (true) {

        // Catch comparison of element first to prevent infinite loop when caught as objects
        case ( _.isElement(obj1[o]) ) :
          if (obj1[o] !== obj2[o]) return false;
          break;
        case ( _.isNaN(obj1[o]) ) :
          if (!_.isNaN(obj2[o])) return false;
          break;
        case ( typeof obj1[o] === "object" ) :
          if (!_.isEqual(obj1[o], obj2[o])) return false;
          break;
        case ( typeof obj1[o] === "function" ) :
          if (!_.isFunction(obj2[o])) return false;
          if (obj1[o].toString() !== obj2[o].toString()) return false;
          break;
        default :
          if (obj1[o] !== obj2[o]) return false;
      }
    }

    // Reverse comparison of `obj2`
    for (var o in obj2) {
      if (typeof obj1 === "undefined") return false;
      if (obj1 === null || obj1 === undefined) return false;
      if (_.isFalsy(obj1[o])) {
        if (_.isNaN(obj1[o])) {
          if (!_.isNaN(obj2[o])) return false;
        } else if (obj1[o] !== obj2[o]) return false;
      }
    }
  }
  return true;
};

I'm looking for advice on strategies I can use to improve its performance. My initial thoughts are get rid of my calls to internal library methods? Should I implement a different type of loop. Would a while lead to any performance gains?


After taking much of what @Joseph the Dreamer said into account I refactored _.isEqual in a seemingly much more efficient manner, which now compares the properties and values that might be attached to functions as well. As you can see I did away with a great many of my previous comparisons because they're rendered redundant in this iteration (I believe).

Here are the unit tests to back this method up: _.isEqual Unit Tests

The code:

_.isEqual = function (obj1, obj2) {
  var type = _.type(obj1), result, o;

  switch (true) {

    // Not the same TYPE
    case type != _.type(obj2) :
      return false;

    // NaNs
    case type == 'nan' :
      return _.isNaN(obj1) && _.isNaN(obj2);

    // Primitives (types that will compare correctly with ===)
    case ((typeof obj1 != 'object' && type != 'function') || type == 'null') :
      return obj1 === obj2;

    // Functions or Elements
    case type == 'function' || type == 'element' :
      return obj1.constructor === obj2.constructor;

    // JavaScript Objects
    default :
      if (_.len(obj1) == 0) {
        result = _.len(obj2) == 0;
      } else {
        for (o in obj1) {
          if (!(result = _.isEqual(obj1[o], obj2[o]))) break;
        }
      }
  }

  return result;
};
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

After going through the code and the library itself, here are some thoughts:

  • Functions are still objects. You can even attach properties to it. You might want to look into this unless you only want to compare the function, excluding the things attached to it.

    function foo(){};
    foo.bar = 'baz';
    console.log(foo.bar); //baz
    
  • Early on, you can weed out values that are equal or actually the same to avoid going through everything. Equal strings, numbers, booleans and even objects in general (Since they are compared by identity) return true.

    obj1 === obj2
    

    If this returns false, it could be that the values are non-equal primitives, or non-identical objects. You can return false for the primitives. For objects in general, only then will you do the "structure equality check".

  • Comparisons return a boolean value. So if you return a boolean after a comparison without doing anything else after, you can shorten the statement.

    /*from*/ if(a !== b) return false;
    /* to */ return a === b;
    
  • While for(var o in obj1) is perfectly valid JS, it's something you'd look out for. You actually did this twice in the code. If you pass your code through a linter, you'd get a o already defined error. Do this instead

    var o;
    
    for(o in obj1){...}
    ...
    for(o in obj2){...}
    

    Or better, just use another variable name instead of o on the second loop.

  • You seem to already have a lot of abstractions in your library, which is good. However, your code seems to use a mix of abstracted and vanilla code. I suggest using your abstracted code so that in case the approach for a certain operation is wrong, you only need to modify the abstracted version.

    jQuery actually does this. It keeps a "Core API" which is used by the other parts of the source. If you look at their source, each and extend are used quite a lot.

  • null and undefined are already falsy values. Assuming you already weeded out other values for obj1, you could already simplify.

    if(typeof obj1 === "undefined") return false;
    if(obj1 === null || obj1 === undefined) return false;
    
    //to
    
    if(!obj1) return false;
    //or
    if(_.isFalsy(obj1)) return false;
    
\$\endgroup\$
3
  • 1
    \$\begingroup\$ I took much of what you said under advisement and posted a revised iteration of the method above. Thank you! ... I'll post the results of the performance tests soon. \$\endgroup\$
    – Xaxis
    Commented Jun 4, 2013 at 15:21
  • \$\begingroup\$ Initial performance tests are looking quite good: jsperf.com/boiler-isequal-vs-underscore-isequal \$\endgroup\$
    – Xaxis
    Commented Jun 5, 2013 at 15:14
  • \$\begingroup\$ @Xaxis You might want to run the unit tests from other libraries to your own implementation, just to be sure. Maybe your code took shortcuts for optimization which other libraries didn't take for compatibility, crash-proofing or some other reason. But the tests look good. \$\endgroup\$
    – Joseph
    Commented Jun 5, 2013 at 15:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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