Your second if
statements don't need to be nested, as there's only two possible exits (return true
or fall through and recurse), so I'd combine them into one:
if (node.children != null && node.splitAttribute == attribute ) {
return true;
}
To me this looks more like a function that takes in a Node
object as a parameter, rather than being a method on a Node
object, which I think it possibly could/should be:
public class Node {
private Node parent;
private Object children;
private int splitAttribute;
//...
//Existing state and behaviour of Node class here
//...
public boolean alreadyHasAttribute(int attribute) {
if (this.parent == null) {
return false;
}
if (this.children != null && this.splitAttribute == attribute) {
return true;
}
return this.parent.alreadyHasAttribute(attribute);
}
}
Then, whenever you have an instance of a Node
, you can replace all your existing calls to alreadyUsed(node, attribute)
with node.alreadyHasAttribute(attribute)
, like in the following (entirely made up) example:
public class Foo {
public static void main(String[] args) {
Node node = getANodeSomehow();
//if (alreadyUsed(node, attribute)) { /*This old approach isn't used anymore*/
if (node.alreadyHasAttribute(attribute)) {
//Do whatever you previously did
//...
}
}
//...
}