HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Empty block in recursion base case

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
caserecursionblockemptybase

Problem

I wrote a recursive function to uncheck all menu nodes. It works well, but I need to know if it is right to leave empty block inside if (nodes[i].children.length === 0) base case.

Is there a better design choice?

The function:

function uncheck_all_tree (nodes) {
  var i, n, n_dom_el;

  for (i = 0; i < nodes.length; i++) {
    if (nodes[i].children.length === 0) {
      // recursion bottom;
    } else {
      uncheck_all_tree(nodes[i].children); 
    }

    // pop nodes from recursion stack
    n = tree.jstree('get_node', nodes[i]);
    n_dom_el = document.getElementById(n.id + '_anchor');
    if (n_dom_el !== null) {
      console.log(n.id + '_anchor');
      n_dom_el.className = 'jstree-anchor';
    }
    n.state.checked = false;
  }
}

Solution

More things that can be improved:
Variable declarations

To allow for easier understanding you should declare variables as close as possible to their usage (even though it's semantically no different)

Additionally IIRC javascript variables are usually camelCase and not snake_case. Furthermore it's useful to write out variable names instead of shortening them as much as possible. This reduces cognitive load and makes the code easier to read.
General case

Simply through the way your code works, I'm dead sure that you don't even need that if-statement you have there. It's superfluous, because it's already working for the general case. If your nodes have 0 elements, the for-loop is not executed and the "recursion bottom" is reached.
"Final" result:

function uncheck_all_tree (nodes) {
    for (var i = 0, length = nodes.length; i < length; i++) {
        uncheck_all_tree(nodes[i].children); 
        var node = tree.jstree('get_node', nodes[i]);
        var nDomElement = document.getElementById(n.id + '_anchor');
        if (nDomElement !== null) {
            console.log(node.id + '_anchor');
            nDomElement.className = 'jstree-anchor';
        }
        node.state.checked = false;
    }
}


In addition to that Ismael Miguel has suggested in a comment to use Array.forEach.call(nodes[i].children, function(node) { [...] }); to further improve performance. More information on how that works can be found at the MDN Documentation

Code Snippets

function uncheck_all_tree (nodes) {
    for (var i = 0, length = nodes.length; i < length; i++) {
        uncheck_all_tree(nodes[i].children); 
        var node = tree.jstree('get_node', nodes[i]);
        var nDomElement = document.getElementById(n.id + '_anchor');
        if (nDomElement !== null) {
            console.log(node.id + '_anchor');
            nDomElement.className = 'jstree-anchor';
        }
        node.state.checked = false;
    }
}

Context

StackExchange Code Review Q#139424, answer score: 7

Revisions (0)

No revisions yet.