patternjavascriptMinor
Empty block in recursion base case
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
Is there a better design choice?
The function:
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
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
"Final" result:
In addition to that Ismael Miguel has suggested in a comment to use
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 DocumentationCode 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.