6
\$\begingroup\$

I have the following JSON list of objects, and I'm able to find the required object, and pass it back to the caller, given the id. Everything works fine, I'm just wondering if there's a better (more efficient) way to return the node. I'm open to using a 3rd party tool like lodash.

JSON payload:

{
  "_expanded": true,
  "_canDrop": false,
  "_id": "-1",
  "_name": "root",
  "_children": [
    {
      "_expanded": true,
      "_canDrop": false,
      "_id": "1",
      "_name": "Child 1",
      "_children": [
        {
          "_expanded": true,
          "_canDrop": false,
          "_id": "1-1",
          "_name": "Child 1-1",
          "_children": [
            {
              "_expanded": false,
              "_canDrop": false,
              "_id": "1-1-1",
              "_name": "Child 1-1-1",
              "_children": []
            }
          ]
        },
        {
          "_expanded": false,
          "_canDrop": false,
          "_id": "1-2",
          "_name": "Child 1-2",
          "_children": []
        },
        {
          "_expanded": false,
          "_canDrop": false,
          "_id": "1-3",
          "_name": "Child 1-3",
          "_children": []
        }
      ]
    },
    {
      "_expanded": true,
      "_canDrop": false,
      "_id": "2",
      "_name": "Child 2",
      "_children": [
        {
          "_expanded": false,
          "_canDrop": false,
          "_id": "2-2",
          "_name": "Child 2-2",
          "_children": []
        }
      ]
    }
  ]
}

Find method:

public findNode = (id: any): TreeNode => {
    let result = null;
    if (this._id === id) {
      result = this;
    } else {
      if (this._children.length > 0) {
        for (let index = 0; index <= this._children.length - 1; index++) {
          result = this._children[index].findNode(id);
          if (result) {
            break;
          }
        }
      }
    }
    return result;
  }
\$\endgroup\$
0
4
\$\begingroup\$

Your method already looks good! I would only make a couple small changes to the logic as follows:

  1. The if (this._children.length > 0) isn't necessary as the for loop will make the check for you
  2. Instead of using a standard for loop, I would prefer a for..of loop. Typescript will transpile this so it will work in any environment
  3. I prefer early returns to nested statements. Because of this, I would recommend storing the result variable in as small a block as possible

With these changes:

public findNode = (id: any): TreeNode => {
  if (this._id === id) {
    return this;
  }

  for (const child of this._children) {
    const result = child.findNode(id);
    if (result) {
      return result;
    }
  }

  return null;
}

Also, here's a couple Typescript specific notes:

  1. Avoid any like the plague. When you use any you are telling typescript to effectively ignore any mistakes in your code. Given your data structure, it looks like id should be of type string
  2. I highly recommend turning on strictNullChecks. This will help prevent a large class of errors
  3. Methods are public by default in Typescript, with this in mind I recommend dropping the public modifier in most cases as it is redundant. However, this does not apply if you are working on a team with developers who generally work with other languages as it could lead to confusion in the team. For example, in C# the default is that all methods are private

With these changes:

findNode = (id: string): TreeNode | null => {
  if (this._id === id) {
    return this;
  }

  for (const child of this._children) {
    const result = child.findNode(id);
    if (result) {
      return result;
    }
  }

  return null;
}
\$\endgroup\$
2
  • \$\begingroup\$ Ah, I see what you mean. The return statement at each condition level is far more elegant. It works really well - many thanks @Gerrit0. As a matter of interest though, what would the TreeNode | null => accomplish? \$\endgroup\$
    – Ruaghain
    Mar 18 '18 at 19:18
  • 1
    \$\begingroup\$ The TreeNode | null only makes sense if you turn on strictNullChecks. If you turn on strictNullChecks and don't explicitly define the return type as potentially being null the compiler won't be happy. \$\endgroup\$
    – Gerrit0
    Mar 18 '18 at 19:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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