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

PrettyPrint a Binary Tree

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

Problem

I was going through this tutorial for Pretty Printing a binary search tree and decided to implement my own version. Here is my implementation. This version works only for trees that are complete binary tree. I would like to know what all optimisations can be done (errors present can be removed) in the code.

```
public class PrettyPrintTree {

private List listOfNodes;
public TreeNode root;

public PrettyPrintTree(List list) {
listOfNodes = list;
root = createTree(listOfNodes);
}

public static class TreeNode {
TreeNode left;
TreeNode right;
int value;

public TreeNode(int value) {
this.value = value;
}
}

public static TreeNode createTree(List list) {
TreeNode root = null;
TreeNode temp, temp2;
for (Integer integer : list) {
if (root == null) {
root = new TreeNode(integer);
root.left = null;
root.right = null;
continue;
}
temp = root;
temp2 = root;
while (temp != null) {
temp2 = temp;
temp = (temp.value rightHeight) ? leftHeight + 1 : rightHeight + 1;
}

private static String getStartingSpace(int height) {
int noOfSpaces = ((int) Math.pow(2, height - 1)) / 2;

StringBuilder startSpaceStringBuilder = new StringBuilder();
for (int i = 0; i queueOfNodes,
int noOfNodesAtCurrentHeight, int height) {
StringBuilder nodesAtHeight = new StringBuilder();
String startSpace = getStartingSpace(height);
String spaceBetweenTwoNodes = getSpaceBetweenTwoNodes(height);
String underScore = getUnderScores(height);

nodesAtHeight.append(startSpace);

for (int i = 0; i queueOfNodes,
int noOfNodesAtCurrentHeight, int height) {
if (height queueOfNodes = new LinkedList<>();
int height = getMaximum

Solution

The private field listOfNodes can be converted to a local variable,
and actually, it can also be inlined in the constructor:

public PrettyPrintTree(List list) {
    root = createTree(list);
}


Whenever possible, you should declare variable types with interfaces instead of implementation, for example instead of this:

private static void printNodes(LinkedList queueOfNodes,
                               int noOfNodesAtCurrentHeight, int height) {


It would be better this way:

private static void printNodes(List queueOfNodes,
                               int noOfNodesAtCurrentHeight, int height) {


In the same method, you have this loop:

for (int i = 0; i < noOfNodesAtCurrentHeight; i++) {
        TreeNode node = (TreeNode) queueOfNodes.get(i);


The cast to TreeNode is unnecessary,
as the type of queueOfNodes is guaranteed by the method signature,
so you can simply write as:

TreeNode node = queueOfNodes.get(i);


In the printBranches method, the queueOfNodes parameter is unused,
so you should remove it, changing the method signature to this:

private static void printBranches(int noOfNodesAtCurrentHeight, int height) {


You have this code duplicated in two methods:

StringBuilder spaceBetweenLeftRightStringBuilder = new StringBuilder();
    for (int i = 0; i < noOfNodesBetweenLeftRightBranch; i++) {
        spaceBetweenLeftRightStringBuilder.append("  ");
    }
    return spaceBetweenLeftRightStringBuilder.toString();


It would be better to move this logic to its own method and reuse it.

Actually you have this kind of duplication in multiple other places as well,
when building strings.
You could reduce duplicated segments more aggressively by creating a parameterized string builder that concatenates a string \$N\$ times (the duplicated operation).

for example:

public static String multiplyString(String string, int times) {
    StringBuilder builder = new StringBuilder(string.length() * times);
    for (int i = 0; i < times; ++i) {
        builder.append(string);
    }
    return builder.toString();
}


In the prettyPrintTree method,
the variable initialization int noOfNodesAtCurrentHeight = 0;
is unnecessary, as the variable is always reassigned inside the while loop.
In fact it would be best to declare the variable inside the loop.

Why the line break in the middle of the statement here:

printBranches(noOfNodesAtCurrentHeight, height
                - level);


The statement is short enough (69 characters) without the line break:

printBranches(noOfNodesAtCurrentHeight, height - level);

Code Snippets

public PrettyPrintTree(List<Integer> list) {
    root = createTree(list);
}
private static void printNodes(LinkedList<TreeNode> queueOfNodes,
                               int noOfNodesAtCurrentHeight, int height) {
private static void printNodes(List<TreeNode> queueOfNodes,
                               int noOfNodesAtCurrentHeight, int height) {
for (int i = 0; i < noOfNodesAtCurrentHeight; i++) {
        TreeNode node = (TreeNode) queueOfNodes.get(i);
TreeNode node = queueOfNodes.get(i);

Context

StackExchange Code Review Q#78145, answer score: 4

Revisions (0)

No revisions yet.