patternjavaMinor
Find all nodes in tree, without a sibling
Viewed 0 times
nodeswithoutallsiblingfindtree
Problem
Find all nodes of trees, without siblings.
This question is attributed to GeeksForGeeks.
I'm looking for code reviews, optimizations and best practices.
```
public class PrintNodesWithoutSiblings {
private TreeNode root;
public PrintNodesWithoutSiblings(List items) {
create(items);
}
private void create (List items) {
root = new TreeNode(items.get(0));
final Queue> queue = new LinkedList>();
queue.add(root);
final int half = items.size() / 2;
for (int i = 0; i current = queue.poll();
int left = 2 * i + 1;
int right = 2 * i + 2;
if (items.get(left) != null) {
current.left = new TreeNode(items.get(left));
queue.add(current.left);
}
if (right (items.get(right));
queue.add(current.right);
}
}
}
}
private static class TreeNode {
private TreeNode left;
private T item;
private TreeNode right;
TreeNode(T item) {
this.item = item;
}
}
public List nonSiblingNodes() {
if (root == null) {
throw new IllegalStateException("Empty tree. Tree cannot be empty.");
}
final List list = new ArrayList();
findNonSiblingNodes(root, list);
return list;
}
private void findNonSiblingNodes(TreeNode node, List list) {
if (node == null) {
return;
}
if (node.left != null && node.right != null) {
findNonSiblingNodes(node.left, list);
findNonSiblingNodes(node.right, list);
} else if (node.left != null) {
list.add(node.left.item);
findNonSibl
This question is attributed to GeeksForGeeks.
I'm looking for code reviews, optimizations and best practices.
```
public class PrintNodesWithoutSiblings {
private TreeNode root;
public PrintNodesWithoutSiblings(List items) {
create(items);
}
private void create (List items) {
root = new TreeNode(items.get(0));
final Queue> queue = new LinkedList>();
queue.add(root);
final int half = items.size() / 2;
for (int i = 0; i current = queue.poll();
int left = 2 * i + 1;
int right = 2 * i + 2;
if (items.get(left) != null) {
current.left = new TreeNode(items.get(left));
queue.add(current.left);
}
if (right (items.get(right));
queue.add(current.right);
}
}
}
}
private static class TreeNode {
private TreeNode left;
private T item;
private TreeNode right;
TreeNode(T item) {
this.item = item;
}
}
public List nonSiblingNodes() {
if (root == null) {
throw new IllegalStateException("Empty tree. Tree cannot be empty.");
}
final List list = new ArrayList();
findNonSiblingNodes(root, list);
return list;
}
private void findNonSiblingNodes(TreeNode node, List list) {
if (node == null) {
return;
}
if (node.left != null && node.right != null) {
findNonSiblingNodes(node.left, list);
findNonSiblingNodes(node.right, list);
} else if (node.left != null) {
list.add(node.left.item);
findNonSibl
Solution
Recursion
The null-check in your recursive method is redundant. The preliminary checks and the mid-recursion checks ensure that the method is never called with a null input.
I also really favour a simplified recursive approach, where the recursion checks it's own conditions, instead of pre-checking the recursive conditions before calling the recursion. In this case, it halves the number of recursive calls, halves the list.adds, and reduces by a third the number of if-conditions in your code:
You would call that code with:
It is safe to remove the null-check on the root node, because it can never be null at that point. That's because ....
Bugs
... your code does horrible things in the constructor, which will fail before it gets here:
Your constructor will throw NPE if the items is null, and will throw NoSuchElementException if the List is empty. Thus, the root will never be null in the recursion.
Style
You should use the 'diamond operator' when using Java7 and later. The line above can be just:
Similarly, the line:
can be:
The null-check in your recursive method is redundant. The preliminary checks and the mid-recursion checks ensure that the method is never called with a null input.
I also really favour a simplified recursive approach, where the recursion checks it's own conditions, instead of pre-checking the recursive conditions before calling the recursion. In this case, it halves the number of recursive calls, halves the list.adds, and reduces by a third the number of if-conditions in your code:
private void findNonSiblingNodes(TreeNode node, boolean hasSibling List list) {
if (node == null) {
return;
}
if (!hasSibling) {
list.add(node);
}
findNonSiblingNodes(node.left, node.right != null, list);
findNonSiblingNodes(node.right, node.left != null, list);
}You would call that code with:
public List nonSiblingNodes() {
final List list = new ArrayList();
findNonSiblingNodes(root, true, list);
return list;
}
}It is safe to remove the null-check on the root node, because it can never be null at that point. That's because ....
Bugs
... your code does horrible things in the constructor, which will fail before it gets here:
public PrintNodesWithoutSiblings(List items) {
create(items);
}
private void create (List items) {
root = new TreeNode(items.get(0));
....Your constructor will throw NPE if the items is null, and will throw NoSuchElementException if the List is empty. Thus, the root will never be null in the recursion.
Style
final Queue> queue = new LinkedList>();You should use the 'diamond operator' when using Java7 and later. The line above can be just:
final Queue> queue = new LinkedList<>();Similarly, the line:
final List list = new ArrayList();can be:
final List list = new ArrayList<>();Code Snippets
private void findNonSiblingNodes(TreeNode<T> node, boolean hasSibling List<T> list) {
if (node == null) {
return;
}
if (!hasSibling) {
list.add(node);
}
findNonSiblingNodes(node.left, node.right != null, list);
findNonSiblingNodes(node.right, node.left != null, list);
}public List<T> nonSiblingNodes() {
final List<T> list = new ArrayList<T>();
findNonSiblingNodes(root, true, list);
return list;
}
}public PrintNodesWithoutSiblings(List<T> items) {
create(items);
}
private void create (List<T> items) {
root = new TreeNode<T>(items.get(0));
....final Queue<TreeNode<T>> queue = new LinkedList<TreeNode<T>>();final Queue<TreeNode<T>> queue = new LinkedList<>();Context
StackExchange Code Review Q#56305, answer score: 8
Revisions (0)
No revisions yet.