0
\$\begingroup\$

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;
  };
\$\endgroup\$
2

3 Answers 3

2
\$\begingroup\$

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 be trimAtPath,

    or if trim is always false traversePath

    or if trim is always true pathStrHasLength

    See second rewrite for how the last two names can be used.

  • prop Is an array so the name should be a plural props.

    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 name rootObj or just root 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]);
\$\endgroup\$
0
\$\begingroup\$
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

\$\endgroup\$
-2
\$\begingroup\$

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;
                
        }
        }
    }
};
```
\$\endgroup\$
1
  • 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
    Commented Aug 12, 2021 at 16:19

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.