I'm writing a function in JavaScript which, when given an element, can look for the previous or next node, assuming a depth-first order traversal. If the next element is out of bounds, it should return null.
I could of course use document.querySelectorAll('*')
, find the index of the element, and then move forwards/backwards by one. But this approach seems like overkill, since I would think it would be inefficient to get the entire DOM tree as a depth-first searched structure, when I could theoretically traverse by one element using local information.
I can't shake the feeling that I'm repeating much of the logic. Can anyone tell if there's a better refactored version I'm not seeing?
//returns previous element in a depth-first search traversal
//returns null if at the start of the tree
TreeWalker.prototype.getPreviousElement = function(element) {
var previousElement;
if(element.previousElementSibling === null) {
var parent = element.parentElement;
if($(parent).is(this.rootSelector)) {
previousElement = null;
} else {
previousElement = parent;
}
} else {
var beforeBranch = element.previousElementSibling;
previousElement = this.cycleToPreviousBranch(beforeBranch);
}
return previousElement;
};
TreeWalker.prototype.cycleToPreviousBranch = function(element) {
var previousElement;
if(element.children.length === 0) {
previousElement = element;
} else {
var lastChild = element.lastElementChild;
previousElement = this.cycleToPreviousBranch(lastChild);
}
return previousElement;
};
//returns next element in a depth-first search traversal
//returns null if at the end of the tree
TreeWalker.prototype.getNextElement = function(element) {
var nextElement;
if(element.children.length > 0) {
nextElement = element.children[0];
} else {
nextElement = this.cycleToNextBranch(element);
}
return nextElement;
};
TreeWalker.prototype.cycleToNextBranch = function(element) {
var nextElement;
if(element.nextElementSibling !== null) {
nextElement = element.nextElementSibling;
} else {
var parent = element.parentElement;
if($(parent).is(this.rootSelector)) {
nextElement = null;
} else {
nextElement = this.cycleToNextBranch(parent);
}
}
return nextElement;
};
1 Answer 1
As the comments say, it's not clear what the if($(parent).is(this.rootSelector))
checks are for. You only use one this
property - this.root
- would it be better to have a root variable and then two simple functions rather than prototype functions?
Your own improvements are better, but they can be improved more by returning earlier. You started doing that with the return null;
, but you can do it with all the values, avoiding all local variables and reducing your code indentations. I have also made your code consistently use brackets after all control structures.
//returns previous element in a depth-first search traversal
//returns null if at the start of the tree
TreeWalker.prototype.getPreviousElement = function(element) {
if (element.previousElementSibling === null) {
if (element === this.root) {
return null;
}
return element.parentNode;
}
element = element.previousElementSibling;
while (element.children.length > 0) {
element = element.lastElementChild;
}
return element;
};
//returns next element in a depth-first search traversal
//returns null if at the end of the tree
TreeWalker.prototype.getNextElement = function(element) {
if (element.children.length > 0) {
return element.children[0];
}
while (element.nextElementSibling === null) {
element = element.parentNode;
if (element === this.root) {
return null;
}
}
return element.nextElementSibling;
};
if($(parent).is(this.rootSelector))
checks. For example,getNextElement()
could be much simpler without seeing that incycleToNextBranch()
. So, part of simplifying the code requires understand why you're doing this? It is also these checks that makes your code something other than a plain traverser. \$\endgroup\$