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.

The reason I want to do this is that the JavaScript objects are created from XML elements and if the element has changed I want to be able to detect that change in the JavaScript objects. So I wrote a function which maps out the differences between the first and second object. Can you improve on this function? (mapObjectDifferences, not the helper functions.)

<html>
<head>
<title>MAP OBJECT DIFFERENCES</title>
<script type="text/javascript">

    function isArray(object) {
        return Object.prototype.toString.apply(object) == "[object Array]";
    }

    function getType(variable) {
        var type = typeof variable;
        if (type == "object")
            if (isArray(variable))
                return "array";
        return type;
    }

    function arrayToObject(array) {
        var object = {};
        for (var i = 0; i < array.length; ++i)
            if (array[i] !== "undefined") object[i] = array[i];
        return object;
    }

    function mapObjectDifferences(outerObject, innerObject) {
        var _mapObjectDifferences = function (outerObject, innerObject, parentMap, parentName) {
            var localMap = {};
            for (var outerProp in outerObject) {
                if (outerObject.hasOwnProperty(outerProp)) {
                    var match = false;
                    var outerPropValue = outerObject[outerProp];
                    var outerType = getType(outerPropValue);
                    var result;
                    for (var innerProp in innerObject) {
                        if (innerObject.hasOwnProperty(innerProp)) {
                            var innerPropValue = innerObject[innerProp];
                            var innerType = getType(innerPropValue);
                            if (outerProp == innerProp && outerType == innerType) {
                                match = true;
                                if (outerType == "array" || outerType == "object") {
                                    if (outerType == "array") {
                                        outerPropValue = arrayToObject(outerPropValue);
                                        innerPropValue = arrayToObject(innerPropValue);
                                    }
                                    _mapObjectDifferences(outerPropValue, innerPropValue, localMap, outerProp);
                                }
                                break;
                            }
                        }
                    }
                    if (match == false) {
                        localMap[outerProp] = outerType;
                        if (parentMap)
                            parentMap[parentName] = localMap;
                    }
                    else if (parentMap) {
                        var difChild = false;
                        for (var prop in localMap)
                            if (localMap.hasOwnProperty(prop)) {
                                difChild = true;
                                break;
                            }
                        if (difChild == true)
                            parentMap[parentName] = localMap;
                    }
                }
            }
            return localMap;
        }
        return _mapObjectDifferences(outerObject, innerObject);
    }

    var o1 = {
        val: "level one",
        val2: 1,
        val3: 3,
        m: {
            s2: "this is level two",
            l1: ["a", "b", "c", 1, {a:"a"}],
            ao: [{ x: "1" }, { y: 1 }, { z: 1}]
        },
        n: {
            n1: { abc: 123 }
        }
    };
    var o2 = {
        val: "level on23e",
        val2: 1,

        m: {
            s3: "this is level two",
             l1: ["a", "b", "c"],
            ao: [{ x: 1 }, { y: 1 }, { z: "3"}]
        },
        n: {
            n1: { abc: "abc" }
        }
    }
    var result = mapObjectDifferences(o1, o2);
    debugger;
</script>
</head>
<body>
</body>
</html>
share|improve this question
    
You can simply use the jQuery extends function...(If jQuery is an option) –  gdoron May 2 '12 at 18:08
    
@FelixKling. I voted to close as too localized because of the lack of that site option for off-topic :( –  gdoron May 2 '12 at 18:10
    
Is there a way to challenge the challenge label as being too vague? –  ControlAltDel May 2 '12 at 18:12
    
@ControlAltDel: Community Bulletin event: Help clean up inappropriate and ambiguous tags! meta.stackoverflow.com/questions/128315/… –  Juan Mendes May 2 '12 at 22:43
    
guys, this is really my fault. i left a narrow gap open through which trolls could feel comfortable commenting while not actually doing anything useful at all. your "community" doesn't provide a helpful answer 9 times out of 10. –  anonymous May 3 '12 at 12:49
add comment

3 Answers

The existing suggestions are good for trimming down your current code, but, as always, the most important thing to optimize is the algorithm itself. I took a crack at your problem, as I understand it, and here's the result:

function difference(o1, o2) {
    var k, kDiff,
        diff = {};
    for (k in o1) {
        if (!o1.hasOwnProperty(k)) {
        } else if (typeof o1[k] != 'object' || typeof o2[k] != 'object') {
            if (!(k in o2) || o1[k] !== o2[k]) {
                diff[k] = o2[k];
            }
        } else if (kDiff = difference(o1[k], o2[k])) {
            diff[k] = kDiff;
        }
    }
    for (k in o2) {
        if (o2.hasOwnProperty(k) && !(k in o1)) {
            diff[k] = o2[k];
        }
    }
    for (k in diff) {
        if (diff.hasOwnProperty(k)) {
            return diff;
        }
    }
    return false;
}

Note that I didn't try to mimic your code exactly:

  • Arrays are compared thoroughly, not just on numbered properties.
  • Difference map contains new values themselves, not their types.
  • If there are no differences then the function returns false, not {}.
share|improve this answer
add comment

Replacing conditions with guard clauses would improve readability a little bit. (Flattening Arrow Code)

I would try to build two maps first - one for properties of the first object, and one for the other - then compare the two maps. The key of the map could be the name of the property (in the parentProperty.childProperty format if a property is an other object). The value could be the type of the property.

share|improve this answer
add comment

I was going to post this as a comment to palacsint's answer but it helps to have code blocks.

Because you're using this pattern in several places:

for (var prop in obj) {
  if(obj.hasOwnProperty(prop)) {
    ...
  }
}

One way you can reduce the depth of the "arrowing" is to extract that out into its own function:

function iterateMap(obj, fn, scope) {
  for(prop in obj) {
    if(obj.hasOwnProperty(prop)) {
      fn.call(scope || this, prop);
    }
  }
}

...

iterateMap(obj, function(prop) {
  ...
});
share|improve this answer
    
@palacsint: Oops, sorry! I've always wondered, what does your username mean? –  seand Jan 9 '13 at 21:11
    
Np :) It comes from 'palacsinta' (which means 'pancake' in Hungarian) without the last letter, since IRCnet allows (or allowed a couple of years ago) only nine characters in the nickname. –  palacsint Jan 9 '13 at 22:19
add comment

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.