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

Binary Tree/ Amorphous Binary Tree Creator

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

Problem

I suppose this is a two-part question. The first part is just a simple implementation of a Binary Tree (BTree), with pre-order, post-order, and in-order searches implemented by default. The second is the AmorphousBTreeCreator(amorphous because the trees sit in a sort of limbo until they are assembled into their final form). The advantage to this is the ability to add nodes (sub-trees) in any order, and then create a Binary Tree out of all those sub-trees. I have a few concerns with each, so any feedback would be much appreciated.

Part 1:

```
class BTree
{
private Node head;

public BTree(T head)
{
this.head = new Node(head);
}

public BTree(Node head)
{
this.head = head;
}

public void addLR(T headE, T leftE, T rightE)
{
Node h = head.get(headE);
h.setLeftChild(new Node(leftE));
h.setRightChild(new Node(rightE));
}

public void addL(T headE, T leftE)
{
Node h = head.get(headE);
h.setLeftChild(new Node(leftE));
}

public void addR(T headE, T rightE)
{
Node h = head.get(headE);
h.setRightChild(new Node(rightE));
}

public Node get(T headE)
{
return head.get(headE);
}

public void prettyPrintTree()
{
//BTreePrinter.printNode(head);
}

public void printPreOrder()
{
head.printPreOrder();
}

public void printPostOrder()
{
head.printPostOrder();
}

public void printInOrder()
{
head.printInOrder();
}
}

class Node
{
private Node rightChild;
private Node leftChild;

T self;

public Node(T self)
{
this.self = self;
}

public Node get(T search)
{
if (self.equals(search))
return this;

if (rightChild == null && leftChild == null)
return null;

if (rightChild != null && leftChild == null)
return rightChild.get(search);

if (leftChild != null && rightChild == null)
return leftChild.get(search);

Node rf = rightChild.get(search);

return (rf != null) ? rf : leftChild.get(search);
}

public Node getRightChild()
{
return rightChild;
}

public void setRightChi

Solution

Review for Node and BTree classes. Stay tuned for more.

I know that this is a religious question, but Java people really like OTBS.

class BTree
{
  private Node head;

  public BTree(T head)
  {
    this.head = new Node(head);
  }

  public BTree(Node head)
  {
    this.head = head;
  }


Why is it called add? It sets children, not adds.

public void addLR(T headE, T leftE, T rightE)
  {
    Node h = head.get(headE);
    h.setLeftChild(new Node(leftE));
    h.setRightChild(new Node(rightE));
  }

  public void addL(T headE, T leftE)
  {
    Node h = head.get(headE);
    h.setLeftChild(new Node(leftE));
  }

  public void addR(T headE, T rightE)
  {
    Node h = head.get(headE);
    h.setRightChild(new Node(rightE));
  }


This is unsafe. Your tree can be corrupted by calls to this Node. Consider making Node class immutable.

public Node get(T headE)
  {
    return head.get(headE);
  }


You don't really need this method, do you?

public void prettyPrintTree()
  {
    //BTreePrinter.printNode(head);
  }

  public void printPreOrder()
  {
    head.printPreOrder();
  }

  public void printPostOrder()
  {
    head.printPostOrder();
  }

  public void printInOrder()
  {
    head.printInOrder();
  }
}

class Node
{
  private Node rightChild;
  private Node leftChild;


This is not exactly self per se... Value maybe?

T self;

  public Node(T self)
  {
    this.self = self;
  }


A bad name for an argument, but an okay name for this very method.

public Node get(T search)
  {
    if (self.equals(search))
      return this;

    if (rightChild == null && leftChild == null)
      return null;

    if (rightChild != null && leftChild == null)
      return rightChild.get(search);


At this point leftChild is always null.

if (leftChild != null && rightChild == null)
      return leftChild.get(search);


Consider this piece of code.

Node result = null;
if (leftChild != null) {
  result = leftChild.get(search);
}
if (result == null && rightChild != null) {
  result = rightChild.get(search);
}
return result;


Why do you go right first? It is conventional to traverse left (smaller) child first.

Node rf = rightChild.get(search);

    return (rf != null) ? rf : leftChild.get(search);
  }


ditto mutability

public Node getRightChild()
  {
    return rightChild;
  }

  public void setRightChild(Node rightChild)
  {
    this.rightChild = rightChild;
  }

  public Node getLeftChild()
  {
    return leftChild;
  }

  public void setLeftChild(Node leftChild)
  {
    this.leftChild = leftChild;
  }

  public void printPreOrder()
  {
    System.out.print(self + " ");
    if (leftChild != null)
      leftChild.printPreOrder();
    if (rightChild != null)
      rightChild.printPreOrder();
  }

  public void printPostOrder()
  {

    if (leftChild != null)
      leftChild.printPostOrder();
    if (rightChild != null)
      rightChild.printPostOrder();
    System.out.print(self + " ");
  }

  public void printInOrder()
  {

    if (leftChild != null)
      leftChild.printInOrder();
    System.out.print(self + " ");
    if (rightChild != null)
      rightChild.printInOrder();
  }

  public String toString()
  {


Bad practice. Use self.toString();

return self + "";
  }
}

Code Snippets

class BTree<T>
{
  private Node<T> head;

  public BTree(T head)
  {
    this.head = new Node<T>(head);
  }

  public BTree(Node<T> head)
  {
    this.head = head;
  }
public void addLR(T headE, T leftE, T rightE)
  {
    Node<T> h = head.get(headE);
    h.setLeftChild(new Node<T>(leftE));
    h.setRightChild(new Node<T>(rightE));
  }

  public void addL(T headE, T leftE)
  {
    Node<T> h = head.get(headE);
    h.setLeftChild(new Node<T>(leftE));
  }

  public void addR(T headE, T rightE)
  {
    Node<T> h = head.get(headE);
    h.setRightChild(new Node<T>(rightE));
  }
public Node<T> get(T headE)
  {
    return head.get(headE);
  }
public void prettyPrintTree()
  {
    //BTreePrinter.printNode(head);
  }

  public void printPreOrder()
  {
    head.printPreOrder();
  }

  public void printPostOrder()
  {
    head.printPostOrder();
  }

  public void printInOrder()
  {
    head.printInOrder();
  }
}

class Node<T>
{
  private Node<T> rightChild;
  private Node<T> leftChild;
T self;

  public Node(T self)
  {
    this.self = self;
  }

Context

StackExchange Code Review Q#44139, answer score: 3

Revisions (0)

No revisions yet.