3
\$\begingroup\$

I have the following code. Providing two objects cmd and this.xxx.properties, script detects the only property changed and mutate the input object (cmd).

Script works fine but I would like to know if I could optimize or improve my code for speed.

    _deltaProperties: function (cmd) {

        var dataBefore = this.xxx.properties,
            newData = {};
        for (var prop in cmd.properties) {
            var newValue = cmd.properties[prop],
                oldValue = dataBefore[prop];
            if (newValue !== oldValue) {
                newData[prop] = cmd.properties[prop];
            }
        }
        cmd.properties = newData;
        return cmd;
    },
\$\endgroup\$
2
  • \$\begingroup\$ Can you clarify what needs to be returned/accomplished? At the moment the input is mutated and then also returned. Do you need a list of changed properties OR do you need to mutate the input to only contain the changed properties? \$\endgroup\$
    – RobH
    Jan 13, 2015 at 14:24
  • \$\begingroup\$ I need to mutate the input to only contain the changed properties. Thanks for your time on this :) \$\endgroup\$
    – GibboK
    Jan 13, 2015 at 15:00

1 Answer 1

1
\$\begingroup\$

I did a bit of testing on jsperf which seems to indicate that it would be marginally faster for you to delete the properties you don't need rather than create a new object and replace it. Obviously, your mileage may vary. My test was objects with 6 properties, 3 of which were different.

EDIT : Delete is faster on IE. The new object version is faster on Chrome and FF.

_deltaProperties: function (cmd) {
    var prop;
    for (prop in cmd.properties) {
        if (cmd.properties[prop] === this.xxx.properties[prop]) {
            delete cmd.properties[prop];
        }
    }
    return cmd;
},

If you decided to keep your current code, there are some improvements you could make in terms of styling:

// I personally dislike underscores in variable names
deltaProperties: function (cmd) {
    // JavaScript only has function scope, a lot of developers define all 
    // variables at the top of the function - can help with subtle scope bugs
    var dataBefore = this.xxx.properties,
        newData = {},
        prop,
        newValue,
        oldValue;

    for (prop in cmd.properties) {
        newValue = cmd.properties[prop],
        oldValue = dataBefore[prop];
        if (newValue !== oldValue) {
            // Use newValue rather than accessing member again
            newData[prop] = newValue;
        }
    }
    cmd.properties = newData;
    return cmd;
},
\$\endgroup\$
2
  • \$\begingroup\$ Thanks for your feedback, could you please post your jsperf link? \$\endgroup\$
    – GibboK
    Jan 13, 2015 at 21:56
  • 1
    \$\begingroup\$ @GibboK - jsperf.com/delete-vs-new-object-code-review/3. Having just looked again I read the chart the wrong way round... fail. Your original method is faster on Chrome and FireFox but the delete method is marginally faster on IE. \$\endgroup\$
    – RobH
    Jan 14, 2015 at 8:25

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.