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

1 Answer 1

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
    Commented Mar 18, 2018 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
    Commented Mar 18, 2018 at 19:42

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.