Hello I have this function that looks for nested value 3 levels deep currently
// This handle nested data to be applied to filter option which comes from filterProps.prop values
// This is currently chained 3 level deep I.E clock.props.children to add more just keep chaineding
const handleNestFilterProp = (prop: any, item: any, trim?: boolean) => {
return trim
? prop.length === 1
? item[prop[0]] && item[prop[0]].trim().length > 0
: prop.length === 2
? item[prop[0]][prop[1]].trim().length > 0
: prop.length === 3 && item[prop[0]][prop[1]][prop[2]].trim().length > 0
: prop.length === 1
? item[prop[0]] && item[prop[0]]
: prop.length === 2
? item[prop[0]][prop[1]]
: prop.length === 3 && item[prop[0]][prop[1]][prop[2]];
};
I'm trying to figure out a way to reduce the logic and make it simpler. I thinking if it would be a good idea to use get from Lodash https://lodash.com/docs/4.17.15#get
UPDATE added lodash get to simplify
const handleNestFilterProp = (prop: any, item: any, trim?: boolean) => {
const L1 = _.get(item, prop[0]);
const L2 = _.get(item, [prop[0], prop[1]]);
const L3 = _.get(item, [prop[0], prop[1], prop[2]]);
return trim
? prop.length === 1
? L1 && L1.trim().length > 0
: prop.length === 2
? L2 && L2.trim().length > 0
: prop.length === 3 && L3.trim().length > 0
: prop.length === 1
? L1
: prop.length === 2
? L2
: prop.length === 3 && L3;
};
UPDATE 2
const handleNestFilterProp = (prop: any, item: any, trim?: boolean) => {
const L1 = _.get(item, prop[0]);
const L2 = _.get(item, [prop[0], prop[1]]);
const L3 = _.get(item, [prop[0], prop[1], prop[2]]);
return prop.length === 1
? trim
? L1.trim().length > 0
: L1
: prop.length === 2
? trim
? L2.trim().length > 0
: L2
: prop.length === 3 && trim
? L3.trim().length > 0
: L3;
};
UPDATE 3
const handleNestFilterProp = (prop: any, item: any, trim?: boolean) => {
const nest = [prop[0], prop[1], prop[2]];
const L1 = _.get(item, nest.slice(0, 1));
const L2 = _.get(item, nest.slice(0, 2));
const L3 = _.get(item, nest);
return prop.length === 1
? trim
? L1.trim().length > 0
: L1
: prop.length === 2
? trim
? L2.trim().length > 0
: L2
: prop.length === 3 && trim
? L3.trim().length > 0
: L3;
};
-
1\$\begingroup\$ (Tagging with a programming language is quite enough, including the language name in the title without special reason is frowned upon \$\endgroup\$greybeard– greybeard2021年08月12日 16:12:17 +00:00Commented Aug 12, 2021 at 16:12
-
\$\begingroup\$ Using a boolean as an argument probably means you could have two different methods with more meaningful names, and call then depending on the variable. See this question \$\endgroup\$Miguel Alorda– Miguel Alorda2021年08月12日 19:26:36 +00:00Commented Aug 12, 2021 at 19:26
3 Answers 3
Undefined
Information is missing in regard to the function's possible inputs. I will make a guess as to what the possibilities are in the rewrites
TypeScript
To use TypeScript and gain any benefit you need to define the types of all variables. Using any
is just lazy and degrades the code quality as type syntax just becomes noise.
I will treat the code as if its JavaScript
Names
You are using some poor names. This means one must decipher their content by reading the code. Names should provide the information needed to understand their content and how they are used.
Looking at the line...
const handleNestFilterProp = (prop: any, item: any, trim?: boolean) => {
... here are some suggest improvements
handleNestFilterProp
can betrimAtPath
,or if
trim
is alwaysfalse
traversePath
or if
trim
is alwaystrue
pathStrHasLength
See second rewrite for how the last two names can be used.
prop
Is an array so the name should be a pluralprops
.Yes they are property names but they are used as directions to the property we are after. It is common to call this type of array usage a
path
item
Is OK but rather generic. It is the starting point (root) of the object you are traversing so the namerootObj
or justroot
would be more informative.trim
is a good name. But really should not be part of the function (see second rewrite)
So the line becomes
const traversePath = (path, root, trim) => {
Your question
Taking from
"...for nested value 3 levels deep currently"
with "currently" meaning you want to handle any depth.
Rather than statically check the depth to find the linked object, the function can step along the path using a loop. You assign the root the next object on the path to the variable that holds the current root.
You don't need to slow down your page with a bulky library (like lodash) as the this is very easy to do in JavaScript or TypeScript
Rewrite
We can greatly simplify the function using the above points, however there are some caveats regarding the inputs.
- Assumes path has 1 or more items
- Assumes root can be indexed or has properties
- Assumes the path is valid
Thus we get the function below.
function trimAtPath(path, root, trim) {
var i = 0;
while (i < path.length) { root = root[path[i++]] }
return trim ? root.trim().length > 0 : root;
}
The above is still not right as the return is not evident in the call. We can break it into two functions.
Note I have swapped the argument order of root
and path
Note traversing the path can be done using Array.reduce
const traversePath = (root, path) => path.reduce((root, key) => root[key], root);
const pathStrHasLen = (root, path) => traversePath(root, path).trim().length > 0;
To test the trimmed string length is made when calling the function using a ternary to select the correct function.
const result = (trim ? pathStrHasLen : traversePath)(root, path);
Alternative
You can also write the traversePath
function to have the same signature as lodash.get
using ... (rest parameters)
const traversePath = (root, ...path) => path.reduce((root, key) => root[key], root);
// called using
traversePath(root, ...path);
// or
traversePath(root, path[0], path[1], path[2]);
const handleNestFilterProp = (prop: string, item: any, trim?: boolean) => {
return trim ? _.get(item, prop).trim().length > 0 : _.get(item, prop);
};
_.get just takes 'props.clock.children' and goes deep no matter how deep
For me, using nested conditional expression makes the code very hard to understand, I think you should use if
and switch
statements for readability. It's difficult to understand what your code returns in each case so I only write the structure of the code.
const handleNestFilterProp = (prop: any, item: any, trim?: boolean) => {
if (trim) {
switch (prop.length) {
case 1:
break;
case 2:
break;
case 3:
break;
}
} else {
switch (prop.length) {
switch (prop.length) {
case 1:
break;
case 2:
break;
case 3:
break;
}
}
}
};
```
-
2\$\begingroup\$ (The nested
switch (prop.length) {
looks off, not just for its indentation. And it would see CR@SE still wants a newline after a closing code fence.) \$\endgroup\$greybeard– greybeard2021年08月12日 16:19:23 +00:00Commented Aug 12, 2021 at 16:19
Explore related questions
See similar questions with these tags.