I'm wondering how I can improve my getPath
function (I found it on SO and adjusted it to my needs - biggest change was returning an array, used to be a string).
It takes an object and a value as inputs and returns an array of object keys for the first matching value they find, otherwise false
.
My focus isn't primarily the speed.
Focus is: As short but at the same time as easy to understand as possible without having to write many comments.
I'm using JS before ECMAScript 6
var nestedObj = {
"rrr": {
"ddd": {
"aa bbbb": {
1: 30009463
},
"cc dddd.": {
1: 30010338
},
"e fff": {
1: 30007744
},
"d-Pool": {
1: 30018363,
2: 30017133,
3: 30013107
},
"e e g": {
1: 30011707,
2: 30017137
},
"f f.-f. (f)": {
1: 30012329
},
"g": {
1: 30011894
}
}
}
};
var result = getPath(nestedObj, 30017137);
document.getElementById("result").innerHTML = result;
function getPath(obj, value, path) {
if (typeof obj !== 'object') {
return false;
}
for (var key in obj) {
if (obj.hasOwnProperty(key)) {
var t = path;
var v = obj[key];
//SLICE is super important, otherwise newPath will reference path
var newPath = path ? path.slice() : [];
newPath.push(key);
if (v === value) {
return newPath;
} else if (typeof v !== 'object') {
newPath = t;
}
var res = getPath(v, value, newPath);
if (res) {
return res;
}
}
}
return false;
}
<div id="result"></div>
I wrote that one comment as a reminder to myself because I spent too much time wondering why the function didn't work before figuring this out.
1 Answer 1
Some thoughts:
- This is not truly a recursive function, in that you are not passing the
path
value when first calling the function. This may hint that you should really just recurse privately within the function itself such that you can better control "state" (mainly that ofpath
and whether or not match has been found) and simplify things. - You can design away pretty much all of the temporary variables you use within the loop.
- Why are you making a check for
hasOwnProperty
? It seems like perhaps you are trying to exclude Arrays, Numbers, etc. here? If that is the case, just check for the object's constructor rather thantypeof
up front and get rid of this. - Consider throwing on case where non-object is encountered in input.
- Why mix return types? If you are expecting an array as a result, why not have empty array represent "no result" case?
Putting it all together might yield something like:
function getPath(obj, value) {
if(obj.constructor !== Object) {
throw new TypeError('getPath() can only operate on object with Object as constructor');
}
var path = [];
var found = false;
function search(haystack) {
for (var key in haystack) {
path.push(key);
if(haystack[key] === value) {
found = true;
break;
}
if(haystack[key].constructor === Object) {
search(haystack[key]);
if(found) break;
}
path.pop();
}
}
search(obj);
return path;
/*
Or alternately if you want to keep mixed return
return found ? path : false;
*/
}
var nestedObj = {
"rrr": {
"ddd": {
"aa bbbb": {
1: 30009463
},
"cc dddd.": {
1: 30010338
},
"e fff": {
1: 30007744
},
"d-Pool": {
1: 30018363,
2: 30017133,
3: 30013107
},
"e e g": {
1: 30011707,
2: 30017137
},
"f f.-f. (f)": {
1: 30012329
},
"g": {
1: 30011894
}
}
}
};
console.log(getPath(nestedObj, 30017137));
console.log(getPath(nestedObj, 'no value'));
-
\$\begingroup\$ I mixed return types because this way I could check the outcome simply with
if(result)
instead of something likeif(result.length>0)
- is there a downside to that mixed return? - I'm not sure why I neededhasOwnProperty
but I used to get an error sometimes without it so I kept it there over time. I always have a hard time following what is happening in recursion - yoursearch
is doing that (truly), right? Theif(found) break;
is one of those things we need to stop the recursion smoothly once we find the value, right? Without it the result array would probably become very long? \$\endgroup\$Cold_Class– Cold_Class2017年06月16日 06:36:26 +00:00Commented Jun 16, 2017 at 6:36
undefined
if value can't be found similar to built-in methods. \$\endgroup\$