Problem
I wrote the following code as a way of plucking data from a n-depth JavaScript object.
It works a charm and even appends the parents of the the found item.
I was just wondering if you guys had any ideas on making it better.
//#region function recursiveFind
function recursiveFind(data, key, childKey, match, result) {
///<summary>Returns JS obj and its parents based on a data set, key and match.</summary>
///<param name="data" type="Object" optional="false">the data object you want to find within</param>
///<param name="key" type="String" optional="false">the key for the data you want to match against eg. 'id' will look for data.id</param>
///<param name="childKey" type="String" optional="false">the data object you want to find within</param>
///<param name="match" type="Any" optional="false">the value you wish to search for</param>
///<returns type="Object">returns the found result</returns>
var parents = [];
var depth = 0;
var recurse = function (data, key, childKey, match, result) {
// Build Parents
depth++;
// Check this Object
if (data[key] === match) {
result = data;
}
else if (depth === 1) {
parents.push(data);
}
// If not found check children
if (result === undefined) {
if (data[childKey]) {
$.each(data[childKey], function (k, v) {
if (v[key] === match) {
result = v;
return false;
} else {
result = recurse(v, key, childKey, match, result);
if (result) {
parents.push(v);
return false;
}
}
});
}
}
// Set parents object into the result
if (result) {
result.parents = $.extend([], parents);
}
// Clean up parents object
depth--;
if (depth === 0) {
parents.length = 0;
}
return result;
};
return recurse(data, key, childKey, match, result);
}
//#endregion
Solution
Overal
I think the code looks too busy just to find a key/value match in an object tree-structure, it took a while to figure out which part(s) bothered me.
Doing it twice
You are doing the matching twice, once with (data[key] === match)
and once with (v[key] === match)
, it would be cleaner to leave the second check to recursion.
Function in a function
I am assuming you use a function in a function to keep track of depth
and parents
, which isn’t worth it. You can do this in a single function with an optional parameter.
Naming
match
->matchValue
becausematch
sounds like a boolean to mechildKey
–childrenKey
because the key really points to an arrayresult.parents
-> avoid this, what if the object already has a property calledparents
?
Counter proposal
If you think about it, result can be considered the parent of key, so I would build a function that returns all parents in an array, if you wish you can then have recursiveFind
call that.
function findParents( data, key, matchValue, childrenKey, parents )
{
var i , children , childrenCount;
/* Prevent the need for an inner function by re-passing parents */
parents = parents || [];
parents.push( data )
/* Return immediately if we find a match */
if( data[key] === matchValue )
return parents.reverse();
/* Are there children to inspect ? */
if( !data[childrenKey] || !data[childrenKey].length )
return false;
children = data[childrenKey];
childrenCount = children.length;
/* See if any children have it ? */
for( i = 0 ; i < childrenCount ; i++ )
{
if( parents = findParents( children[i] , key , matchValue , childrenKey , parents.slice() ) )
return parents;
}
/* Fail.. */
return false;
}
Then recursiveFind
becomes
function recursiveFind(data, key, childrenKey, match )
{
var parents = findParents( data , key, match ),
result = parents.shift()
result.parents = parents;
return result;
}