-1
\$\begingroup\$

I asked the same question on StackOverflow but was told to instead ask here since it was better suited. I have a function that I implemented. It is doing a lot of heavy lifting. I would like to optimise it or in other words try to find repeating code and simplify it a little bit. I'll show my implementation below:

setModifiedPaths(limit, assetTypeIndex, categoryIndex, subCategoryIndex = '', subSubCategoryIndex = '') {
    const { modified, deleted, typeObj, typeObjCopy } = this.state;
    let [modifiedArr, oldAssetTypeObj, oldCategoryObj, oldSubcategoryObj, oldSubSubCategoryObj, newAssetTypeObj, newCategoryObj, newSubCategoryObj,
      newSubSubCategoryObj, oldAssetSubSubCategoryPath, newAssetSubSubCategoryPath, oldAssetSubCategoryPath,
      newAssetSubCategoryPath, oldAssetCategoryPath, newAssetCategoryPath] = Array(6).fill('');

    modifiedArr = modified;
    oldAssetTypeObj = typeObjCopy[assetTypeIndex];
    oldCategoryObj = typeObjCopy[assetTypeIndex].category[categoryIndex];
    newAssetTypeObj = typeObj[assetTypeIndex];
    newCategoryObj = typeObj[assetTypeIndex].category[categoryIndex];

    if (!_.isEmpty(subCategoryIndex) && _.has(typeObjCopy[assetTypeIndex].category[categoryIndex], "subCategories")) {
      oldSubcategoryObj = typeObjCopy[assetTypeIndex].category[categoryIndex].subCategories[subCategoryIndex];
      newSubCategoryObj = typeObj[assetTypeIndex].category[categoryIndex].subCategories[subCategoryIndex]
    }

    if (!_.isEmpty(subSubCategoryIndex) && _.has(typeObjCopy[assetTypeIndex].category[categoryIndex].subCategories[subCategoryIndex], "subSubCategories")) {
      oldSubSubCategoryObj = typeObjCopy[assetTypeIndex].category[categoryIndex].subCategories[subCategoryIndex].subSubCategories[subSubCategoryIndex];
      newSubSubCategoryObj = typeObj[assetTypeIndex].category[categoryIndex].subCategories[subCategoryIndex].subSubCategories[subSubCategoryIndex];
    }

    switch (limit) {
      case 'subSubCategory':
        if (!!oldSubSubCategoryObj && !_.isEmpty(oldSubSubCategoryObj.label) && !_.isEmpty(newSubCategoryObj.label) && !_.isEmpty(newCategoryObj)) {
          oldAssetSubSubCategoryPath = oldAssetTypeObj.label + "." + oldCategoryObj.label + "." + oldSubcategoryObj.label + "." + oldSubSubCategoryObj.label;
          newAssetSubSubCategoryPath = oldAssetTypeObj.label + "." + oldCategoryObj.label + "." + oldSubcategoryObj.label;
          modifiedArr = this.rejectSameFromModifiedArr(modifiedArr);
          if (!_.isEmpty(newSubSubCategoryObj.label)) {
            newAssetSubSubCategoryPath = newAssetSubSubCategoryPath + "." + newSubSubCategoryObj.label;
            modifiedArr.push({ oldPath: oldAssetSubSubCategoryPath, newPath: newAssetSubSubCategoryPath });
          } else {
            deleted.push(oldAssetSubSubCategoryPath);
            this.setState({ deleted })
          }
        }
        break;
      case 'subCategory':
        if (!!oldSubcategoryObj && !_.isEmpty(oldSubcategoryObj.label) && !_.isEmpty(newCategoryObj.label) && !_.isEmpty(newAssetTypeObj)) {
          oldAssetSubCategoryPath = oldAssetTypeObj.label + "." + oldCategoryObj.label + "." + oldSubcategoryObj.label;
          newAssetSubCategoryPath = oldAssetTypeObj.label + "." + oldCategoryObj.label;
          modifiedArr = _.reject(modifiedArr, (obj) => {
            return obj.oldPath === oldAssetSubCategoryPath;
          });
          if (!_.isEmpty(newSubCategoryObj.label)) {
            newAssetSubCategoryPath = newAssetSubCategoryPath + "." + newSubCategoryObj.label;
            modifiedArr.push({ oldPath: oldAssetSubCategoryPath, newPath: newAssetSubCategoryPath });
          } else {
            deleted.push(oldAssetSubCategoryPath);
            this.setState({ deleted });
          }
        }
        break;
      case 'category':
        if (!!oldCategoryObj && !_.isEmpty(oldCategoryObj.label) && !_.isEmpty(newAssetTypeObj)) {
          oldAssetCategoryPath = oldAssetTypeObj.label + "." + oldCategoryObj.label;
          newAssetCategoryPath = oldAssetTypeObj.label;
          modifiedArr = _.reject(modifiedArr, (obj) => {
            return obj.oldPath === oldAssetCategoryPath
          });
          if (!_.isEmpty(newCategoryObj.label)) {
            newAssetCategoryPath = newAssetCategoryPath + "." + newCategoryObj.label;
            modifiedArr.push({ oldPath: oldAssetCategoryPath, newPath: newAssetCategoryPath });
          } else {
            deleted.push(oldAssetCategoryPath);
            this.setState({ deleted });
          }
        }
        break;
      default:
        if (!!oldAssetTypeObj && !_.isEmpty(oldAssetTypeObj.label)) {
          let oldAssetTypePath = oldAssetTypeObj.label;
          let newAssetTypePath = newAssetTypeObj.label;
          modifiedArr = _.reject(modifiedArr, (obj) => {
            return obj.oldPath === oldAssetTypePath
          })
          if (!_.isEmpty(newAssetTypePath)) {
            modifiedArr.push({ oldPath: oldAssetTypePath, newPath: newAssetTypePath });
          } else {
            deleted.push(oldAssetTypePath);
            this.setState({ deleted });
          }
        }
        break;
    }
    return modifiedArr;
  }

The variables deleted and modified are empty arrays. The variable typeObjCopy is a deeply cloned copy of typeObj. Let me show a little bit of the contents of typeObj for reference:

[
    {
        "label": "testType",
        "categories": [
            {
                "label": "testCatType",
                "subCategories": [
                    {
                        "label": "testSubCat",
                        "subSubCategories": [
                            {
                                "label": "testSubSubCat1"
                            },
                            {
                                "label": "testSubSubCat2"
                            }
                        ]
                    }
                ]
            }
        ]
    },
    {
        "label": "newType",
        "categories": [
            {
                "label": "newCat10",
                "subCategories": [
                    {
                        "label": "newCatSub1",
                        "subSubCategories": [
                            {
                                "label": "bingo11"
                            },
                            {
                                "label": "bingo12"
                            },
                            {
                                "label": "bingo15"
                            }
                        ]
                    }
                ]
            }
        ]
    },
    {
        "label": "displacement",
        "categories": []
    },
    {
        "label": "imperfection",
        "categories": [
            {
                "label": "metal",
                "subCategories": [
                    {
                        "label": "clean",
                        "subSubCategories": []
                    },
                    {
                        "label": "scratched",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "dust",
                "subCategories": []
            },
            {
                "label": "leakage",
                "subCategories": []
            },
            {
                "label": "wipe mark",
                "subCategories": []
            },
            {
                "label": "fingerprint",
                "subCategories": []
            },
            {
                "label": "grunge",
                "subCategories": []
            },
            {
                "label": "other",
                "subCategories": []
            },
            {
                "label": "rubber",
                "subCategories": []
            },
            {
                "label": "grain",
                "subCategories": []
            },
            {
                "label": "stone",
                "subCategories": []
            },
            {
                "label": "stain",
                "subCategories": []
            }
        ]
    },
    {
        "label": "surface",
        "categories": [
            {
                "label": "metal",
                "subCategories": [
                    {
                        "label": "bare",
                        "subSubCategories": []
                    },
                    {
                        "label": "corroded",
                        "subSubCategories": []
                    },
                    {
                        "label": "corrugated",
                        "subSubCategories": []
                    },
                    {
                        "label": "gun",
                        "subSubCategories": []
                    },
                    {
                        "label": "painted",
                        "subSubCategories": []
                    },
                    {
                        "label": "sheet",
                        "subSubCategories": []
                    },
                    {
                        "label": "treated",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "wood",
                "subCategories": [
                    {
                        "label": "board",
                        "subSubCategories": []
                    },
                    {
                        "label": "log",
                        "subSubCategories": []
                    },
                    {
                        "label": "other",
                        "subSubCategories": []
                    },
                    {
                        "label": "parquet",
                        "subSubCategories": []
                    },
                    {
                        "label": "plank",
                        "subSubCategories": []
                    },
                    {
                        "label": "veneer",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "fabric",
                "subCategories": [
                    {
                        "label": "carpet",
                        "subSubCategories": []
                    },
                    {
                        "label": "leather",
                        "subSubCategories": []
                    },
                    {
                        "label": "pattern",
                        "subSubCategories": []
                    },
                    {
                        "label": "plain",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "grass",
                "subCategories": [
                    {
                        "label": "artificial",
                        "subSubCategories": []
                    },
                    {
                        "label": "dried",
                        "subSubCategories": []
                    },
                    {
                        "label": "lawn",
                        "subSubCategories": []
                    },
                    {
                        "label": "patchy",
                        "subSubCategories": []
                    },
                    {
                        "label": "wild",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "concrete",
                "subCategories": [
                    {
                        "label": "cast in situ",
                        "subSubCategories": []
                    },
                    {
                        "label": "damaged",
                        "subSubCategories": []
                    },
                    {
                        "label": "dirty",
                        "subSubCategories": []
                    },
                    {
                        "label": "painted",
                        "subSubCategories": []
                    },
                    {
                        "label": "rough",
                        "subSubCategories": []
                    },
                    {
                        "label": "slab",
                        "subSubCategories": []
                    },
                    {
                        "label": "smooth",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "sand",
                "subCategories": [
                    {
                        "label": "beach",
                        "subSubCategories": []
                    },
                    {
                        "label": "desert",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "stone",
                "subCategories": [
                    {
                        "label": "castle",
                        "subSubCategories": []
                    },
                    {
                        "label": "cobblestone",
                        "subSubCategories": []
                    },
                    {
                        "label": "floor",
                        "subSubCategories": []
                    },
                    {
                        "label": "mosaic",
                        "subSubCategories": []
                    },
                    {
                        "label": "terrazzo",
                        "subSubCategories": []
                    },
                    {
                        "label": "wall",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "plaster",
                "subCategories": [
                    {
                        "label": "damaged",
                        "subSubCategories": []
                    },
                    {
                        "label": "fresh",
                        "subSubCategories": []
                    },
                    {
                        "label": "old",
                        "subSubCategories": []
                    },
                    {
                        "label": "painted",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "soil",
                "subCategories": [
                    {
                        "label": "clay",
                        "subSubCategories": []
                    },
                    {
                        "label": "mud",
                        "subSubCategories": []
                    },
                    {
                        "label": "mulch",
                        "subSubCategories": []
                    },
                    {
                        "label": "sandy",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "rock",
                "subCategories": [
                    {
                        "label": "cliff",
                        "subSubCategories": []
                    },
                    {
                        "label": "granite",
                        "subSubCategories": []
                    },
                    {
                        "label": "jagged",
                        "subSubCategories": []
                    },
                    {
                        "label": "lava",
                        "subSubCategories": []
                    },
                    {
                        "label": "mossy",
                        "subSubCategories": []
                    },
                    {
                        "label": "rough",
                        "subSubCategories": []
                    },
                    {
                        "label": "smooth",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "moss",
                "subCategories": [
                    {
                        "label": "ground",
                        "subSubCategories": []
                    },
                    {
                        "label": "rock",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "debris",
                "subCategories": [
                    {
                        "label": "construction",
                        "subSubCategories": []
                    },
                    {
                        "label": "nature",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "brick",
                "subCategories": [
                    {
                        "label": "modern",
                        "subSubCategories": []
                    },
                    {
                        "label": "mortar",
                        "subSubCategories": []
                    },
                    {
                        "label": "painted",
                        "subSubCategories": []
                    },
                    {
                        "label": "rough",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "tile",
                "subCategories": [
                    {
                        "label": "ceramic",
                        "subSubCategories": []
                    },
                    {
                        "label": "grout",
                        "subSubCategories": []
                    },
                    {
                        "label": "pavestone",
                        "subSubCategories": []
                    },
                    {
                        "label": "sidewalk",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "asphalt",
                "subCategories": [
                    {
                        "label": "fine",
                        "subSubCategories": []
                    },
                    {
                        "label": "rough",
                        "subSubCategories": []
                    },
                    {
                        "label": "torn",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "other",
                "subCategories": [
                    {
                        "label": "climber",
                        "subSubCategories": []
                    },
                    {
                        "label": "creature",
                        "subSubCategories": []
                    },
                    {
                        "label": "dirt road",
                        "subSubCategories": []
                    },
                    {
                        "label": "edible",
                        "subSubCategories": []
                    },
                    {
                        "label": "fur",
                        "subSubCategories": []
                    },
                    {
                        "label": "paper",
                        "subSubCategories": []
                    },
                    {
                        "label": "various",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "snow",
                "subCategories": [
                    {
                        "label": "mixed",
                        "subSubCategories": []
                    },
                    {
                        "label": "pure",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "bark",
                "subCategories": [
                    {
                        "label": "beech",
                        "subSubCategories": []
                    },
                    {
                        "label": "birch",
                        "subSubCategories": []
                    },
                    {
                        "label": "oak",
                        "subSubCategories": []
                    },
                    {
                        "label": "other",
                        "subSubCategories": []
                    },
                    {
                        "label": "palm",
                        "subSubCategories": []
                    },
                    {
                        "label": "pine",
                        "subSubCategories": []
                    },
                    {
                        "label": "willow",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "gravel",
                "subCategories": [
                    {
                        "label": "construction",
                        "subSubCategories": []
                    },
                    {
                        "label": "natural",
                        "subSubCategories": []
                    },
                    {
                        "label": "pebbledash",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "marble",
                "subCategories": [
                    {
                        "label": "polished",
                        "subSubCategories": []
                    },
                    {
                        "label": "rough",
                        "subSubCategories": []
                    },
                    {
                        "label": "tile",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "ground",
                "subCategories": [
                    {
                        "label": "forest",
                        "subSubCategories": []
                    },
                    {
                        "label": "jungle",
                        "subSubCategories": []
                    },
                    {
                        "label": "other",
                        "subSubCategories": []
                    },
                    {
                        "label": "roots",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "roofing",
                "subCategories": [
                    {
                        "label": "new",
                        "subSubCategories": []
                    },
                    {
                        "label": "old",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "antique",
                "subCategories": [
                    {
                        "label": "asian",
                        "subSubCategories": []
                    },
                    {
                        "label": "medieval",
                        "subSubCategories": []
                    },
                    {
                        "label": "middle-eastern",
                        "subSubCategories": []
                    },
                    {
                        "label": "roman",
                        "subSubCategories": []
                    }
                ]
            },
            {
                "label": "coal",
                "subCategories": [
                    {
                        "label": "brick",
                        "subSubCategories": []
                    },
                    {
                        "label": "debris",
                        "subSubCategories": []
                    }
                ]
            }
        ]
    },
    {
        "label": "brush",
        "categories": [
            {
                "label": "blood",
                "subCategories": []
            },
            {
                "label": "damage",
                "subCategories": []
            },
            {
                "label": "grunge",
                "subCategories": []
            },
            {
                "label": "leakage",
                "subCategories": []
            },
            {
                "label": "print",
                "subCategories": []
            },
            {
                "label": "spatter",
                "subCategories": []
            },
            {
                "label": "sponge",
                "subCategories": []
            },
            {
                "label": "spray",
                "subCategories": []
            },
            {
                "label": "stain",
                "subCategories": []
            },
            {
                "label": "traditional",
                "subCategories": []
            }
        ]
    }
]

As you can see it has grown pretty out of proportion and I would like it to be more readable. Before this implementation, the previous implementation was even more convoluted and I have so far been able to compress it to this logic.

Would really appreciate any help and suggestions!

\$\endgroup\$
6
  • 2
    \$\begingroup\$ What does the code do? \$\endgroup\$ Jul 29 at 10:39
  • \$\begingroup\$ @MagnusJeffsTovslid so I have mapped the typeObj values as input fields and when someone changes those field values, I track what is being changes. I have to send an array to the backend containing new and old paths formed because of those changes. So an example output of this function would be: [ { newPath: "testType.categoryTypeChanged" oldPath: "testType.testCatType" } ] \$\endgroup\$ Jul 29 at 10:45
  • \$\begingroup\$ I think I need a lot more context and info to understand what this is supposed to do. Is it just checking if a value inside some path has changed? If so, I would think you could simplify this greatly by having the same "type" of object all the way down. E.g. not call it subcategory and subsubcategory, but just category all the way. Then you could make a simple recursive algorithm to traverse it. But like I said, I would need more info. \$\endgroup\$ Jul 29 at 10:57
  • \$\begingroup\$ @MagnusJeffsTovslid so I have to keep the nomenclature intact because if you see the structure of typeObj it is a nested object. So the parent key is considered an asset type. Each asset type can have multiple categories. each category can have multiple sub categories. Each sub category can have multiple sub sub categories. And so let's say I changed the value of a sub sub category belonging to a sub category, I need to construct the modifiedArr collection containing info about newPath and oldPath. I hope I am making some sense here! \$\endgroup\$ Jul 29 at 11:04
  • 2
    \$\begingroup\$ Welcome to Code Review! We need to know what the code is intended to achieve. To help reviewers give you better answers, please add sufficient context to your question, including a title that summarises the purpose of the code. We want to know why much more than how. The more you tell us about what your code is for, the easier it will be for reviewers to help you. The title needs an edit to simply state the task, rather than your concerns about the code. \$\endgroup\$ Jul 29 at 11:53
1
\$\begingroup\$

I'm gonna guess that the code is really trying to find the diff between two tree structures. Futher, I'm assuming from the name "typeObjCopy" that the two trees have identical structure, just different labels. I.e. no added categories allowed. Hopefully this will still be helpful for you even if I'm not entirely correct about this.

When I saw the "typeObj" object with categories and sub categories etc, I immediately expected a recursive solution. It is a bit hampered by the fact that each level in the tree have a different name for the children. But other than that, we have only labels to deal with. So to avoid all the repeating code it would be extremely helpful to rewrite this into a recursion, imo.

To see how the current solution is repetitive is a little bit annoying with the way it is written. For example, you declare a bunch of variables at the top which are only used for one of the cases each. This requires that the variables have unique names, which makes them look different. If you instead declare those variables inside of each case, you can make them have the same name in all of the cases, and the duplication is obvious. To sum up: declare variables as close to the usage as possible.

I would also try to separate this type of complex logic from react state handling. In this case it's pretty easy to make it a pure function, and then it is very easy to test as well.

Below is an example of a recursive solution. I'm not sure if it solves your problem exactly, but I think something like this is what you're looking for.

const childrenByLevel = {
    0: 'categories',
    1: 'subCategories',
    2: 'subSubCategories'
}

function diff(nodesOld, nodesNew, prevOldPath = '', prevNewPath = '', level = 0) {
    const modified = []
    const deleted = []

    for (let i = 0; i < nodesOld.length; i++) {
        const nodeOld = nodesOld[i]
        const nodeNew = nodesNew[i]

        const oldPath = [prevOldPath, nodeOld.label].filter(x => !!x).join('.')
        const newPath = [prevNewPath, nodeNew.label].filter(x => !!x).join('.')

        if (!nodeNew.label) {
            deleted.push(oldPath)
        } else {
            if (nodeOld.label !== nodeNew.label) {
                modified.push({ oldPath, newPath })
            }

            const childrenOld = nodeOld[childrenByLevel[level]]
            const childrenNew = nodeNew[childrenByLevel[level]]

            if (!childrenOld || !childrenNew) continue

            const result = diff(childrenOld, childrenNew, oldPath, newPath, level + 1)

            modified.push(...result.modified)
            deleted.push(...result.deleted)
        }
    }

    return { modified, deleted }
}

console.log(diff(typeObj, typeObjNew))
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.