Object key diff

Question

I've been tasked with finding property differences in massive JSON structures and returning them in a particular fashion (i.e. [key_1.key_2, key_1, etc.]).

Here's my code, which is focused on readability so some redundant decorators are used (they will be removed). I am looking for any performance/logical problems with this approach.

It also has to be in JavaScript, Node Stack (Isomorphic).

/**
 * --Decorator for prettiness-- 
 * Checks array if key exists
 * @param needle
 * @param haystack
 * @returns {boolean}
 */
export function inArray (needle, haystack) {
 return haystack.indexOf(needle) > -1
}
/**
 * --Decorator for prettiness-- 
 * Merge two arrays together
 * @param array_1 {Array}
 * @param array_2 {Array}
 * @returns {Array}
 */
export function arrayMerge(array_1, array_2) {
 return array_1.concat(array_2)
}
/**
 * Returns all differences of keys
 * Note: nested keys are returned like this [key_1.key_2, key_1.key_2.key_3]
 * @param object_compare
 * @param object_against
 * @param ignoreKeys
 * @param depth
 * @returns {Array}
 */
export function diffKeysRecursive(object_compare, object_against, ignoreKeys = [], depth = '') {
 function _isObject(mixed) {
 return typeof mixed === 'object'
 }
 function _propExists(prop, object) {
 return typeof object === 'object' && typeof object[prop] !== 'undefined'
 }
 function _depthPath(prop, depth = '') {
 return depth = depth ? `${depth}.${prop}` : prop
 }
 if (_isObject(object_against)) {
 var diffs = []
 // Iterate through the against object to see differences in the compare
 $.each(object_against, (prop, value_against) => {
 // Checks if prop should be ignored
 if(!inArray(prop, ignoreKeys)) {
 if (!_propExists(prop, object_compare)) {
 diffs.push(_depthPath(prop, depth))
 }
 // Recurse if value_against is an object
 if (_isObject(value_against)) {
 var object_compare_safe = _propExists(prop, object_compare) ? object_compare[prop] : null
 diffs = arrayMerge(diffs, diffKeysRecursive(object_compare_safe, value_against, _depthPath(prop, depth)))
 }
 }
 })
 return diffs
 } else {
 throw 'Invalid Type Supplied'
 }
}

Answer 1

$.each(object_against, (prop, value_against) => {

I assume this is jQuery? To make the code more portable, I suggest dropping the dependency to jQuery and simply use a combination of Object.keys and forEach.

Object.keys(object_against).forEach(prop => {
 var value_against = object_against[prop];
 // The rest of the code here.
});

---

You also need to be consistent with your naming scheme. JavaScript uses camelCase syntax. You're already doing it for function names, but you need to do the same for variable names.

---

export function arrayMerge(array_1, array_2) {
 return array_1.concat(array_2)
}

This is redundant since it's already built-in. If you want a function-like approach, you can simply use Array.prototype.concat directly.

var mergedArrays = Array.prototype.concat.call(array1, array2);

---

export function diffKeysRecursive(object_compare, object_against, ignoreKeys = [], depth = '') {
 function _isObject(mixed) {
 return typeof mixed === 'object'
 }
 function _propExists(prop, object) {
 return typeof object === 'object' && typeof object[prop] !== 'undefined'
 }
 function _depthPath(prop, depth = '') {
 return depth = depth ? `${depth}.${prop}` : prop
 }

Suggesting you pull out the functions out of diffKeysRecursive and into the module as non-exported functions. The problem here is that every call to diffKeysRecursive, it creates these functions. If encapsulation is what you need, the module itself should suffice.

---

 function _isObject(mixed) {
 return typeof mixed === 'object'
 }

typeof null is also object. Refine this by adding a guard for truthiness.

 function _isObject(mixed) {
 return mixed && typeof mixed === 'object'
 }

---

function _propExists(prop, object) {
 return typeof object === 'object' && typeof object[prop] !== 'undefined'
}

This is misleading. A prop can exist with a value of undefined. Suggesting you rename the function to something like isPropUndefined.

Additionally, object[prop] may cause confusion. For example, do ({}).valueOf !== undefined, it will return true. That's because a method named valueOf exists on the prototype. To avoid this, use obj.hasOwnProperty(prop) instead, to check on the current instance and avoid resolving up the prototype.

• Matt Oaxaca: Dang ty, this is exactly what I was looking for! So your point about pulling the functions outside of the function.
• Matt Oaxaca: Is there a performance increase with moving them outside of the scope and hoisting them in? I can kinda see the benefit, assuming it's redeclaring the function on every invocation. I am no expert in Javascripts closure scoping but I might be giving it too much merit.

Answer 2

export function inArray (needle, haystack) {

 return haystack.indexOf(needle) > -1
}

This looks like code that supports browsers like IE9, but your code uses arrow functions and other ecmascript-6 features not supported by those browsers. Since that is the case, includes() can be used to simplify this function slightly:

export function inArray (needle, haystack) {
 return haystack.includes(needle)
}

The performance difference will likely be negligible... This SO answer has a link to this relevant jsPerf test.

One might be able to also simplify the function by converting it to an arrow function (like was used on the callback to $.each()).

---

var diffs = []

As was mentioned in the previous section, ecmascript-6 features are used in this code so instead of using var it is generally advisable to use let or const for any value that should not be re-assigned.