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

BinaryTree class with extra methods

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

Problem

I am taking a data structures course. I would like to check whether my BinaryTree code is perfect, fine or has lots of mistakes.

I would like any kind of advice on this code so I can learn from my mistakes (syntax, techniques, style, etc...).

Note: This is not a binary search tree.

BinaryTree class:

```
/**
* @uniersity Saint Joseph's University
* @college College of Arts and Sciences
* @department Computer Science
* @course CSC 550: Object Oriented Design & Data Structures
*/

package mih1406.csc550.trees;
import java.io.Serializable;

// To make life easier...
// to simplify it if any method requires Integer
// eg: getSum, countLessThan
public class BinaryTree implements Serializable {
// Inner Class
protected class Node implements Serializable {
protected E data;
protected Node left;
protected Node right;

public Node(E data) {
this.data = data;

left = null;
right = null;
}

public String toString() {
return data.toString();
}
}

// Data Fields
protected Node root;

// Constructors
public BinaryTree() {
root = null;
}

protected BinaryTree(Node root) {
this.root = root;
}

public BinaryTree(E data, BinaryTree leftTree,
BinaryTree rightTree) {
root = new Node(data);
if(leftTree == null || leftTree.root == null)
root.left = null;
else
root.left = leftTree.root;

if(rightTree == null || rightTree.root == null)
root.right = null;
else
root.right = rightTree.root;
}

// Methods
public BinaryTree getLeftSubtree() {
if(root == null)
return new BinaryTree(null);
else
return new BinaryTree(root.left);
}

public BinaryTree getRightSubtree() {
if(root == null)
return new BinaryTree(n

Solution

This is a good start. Here's what I have to say about it.

-
There really is no point in making E constrained on Integer. The Integer class is final so it can't be anything but an Integer. Either remove that constraint or remove the use of generics here and make it an Integer.

-
Your inner class could be made static. A node here doesn't need the outer class to mean anything and can exist on its own.

-
Pretty much all members in your classes that are protected probably should be private. Don't get into the habit of using protected over private just because. Only do it because you intend for it to be overridden or used in a derived class.

-
Personally, I wouldn't allow for the possibility of having a null root node. It will help simplify your code so you don't have to put null checks everywhere you need to operate on the root.

-
Your toString() method should return a string representation of a single tree. What you have there (and in other related methods) is the kind of code I would expect to see in a "printTree" method. It doesn't belong there.

-
Consider making your different traversal methods either return an iterator that iterates over the nodes in the appropriate order or use them to apply the visitor pattern on what operations you want to do on your tree. It would make them more useful in general rather than just only for filling a string buffer.

-
Local variables should be in camelCase at most, not Capitalized.

Context

StackExchange Code Review Q#18575, answer score: 3

Revisions (0)

No revisions yet.