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

Refactoring code finding if Binary Tree is BST

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

Problem

I have written an code to find if a given tree is a BST. Need help in refactoring it. Some of the things I am looking are.

  • Getting Rid of temp variables (As suggested by Martin Fowler)



  • Combining leftTreeMax and RighTreeMin methods



  • Better naming of methods.



Also it will be great if someone can suggest other refactorings which i can do.

```
package com.cc.bst;

import com.cc.queue.Queue;
import com.cc.trees.BinaryTreeNode;

public class IsBinaryTreeBST {

public static boolean binaryTreeBST ( BinaryTreeNode root) {

if (root == null) {
return false;
}
int maxVal = leftTreeMax(root.getLeft());
int minVal = rightTreeMin(root.getRight());
int rootVal = root.getData();

if (maxVal == 0 || minVal == 0 ) {
return false;
}

if (rootVal > maxVal && rootVal tempNode.getData()) {
return 0;
}
nodeQueue.enqueue(left);
}
if (right != null) {
if ( tempNode.getData() > right.getData()) {
return 0;
}
nodeQueue.enqueue(right);
}
if (tempNode.getData() > maxValue) {
maxValue = tempNode.getData();
}
}
System.out.println("---------- maxVal -------->" + maxValue);
return maxValue;
}

private static int rightTreeMin(BinaryTreeNode node) {

if (node == null) {
return 0;
}
Queue nodeQueue = new Queue();
nodeQueue.enqueue(node);
int minValue = node.getData();

while (!nodeQueue.isEmpty()) {
BinaryTreeNode tempNode = (BinaryTreeNode) nodeQueue.dequeue();
BinaryTreeNode left = tempNode.getLeft();
BinaryTreeNode right = tempNode.getRight();
System.out.println("---------- tempNode -------->" + tempNode.getData());

if (left != null ) {
System.out.println("---------- left -------->" + left.getData());

if (left.getData() > tempNode.getData()) {
re

Solution

1) You would like to get rid of the System.out.println() inside methods. Noone expects a method to write to the console unless the method has print in the name.

2) Just comparing the root with left tree max and right tree min can lead to wrong result. Consider,

12
   /  \
  9    15
 / \
3   5


Here the left sub-tree is not a BST but 12 > 9 and 15 > 12. You must also ensure that left-subtree and right-subtree of root are also BST.

3) BinaryTreeBST is kind of redundant. Consider naming the class IsBSTCheck or IsBinarySearchTreeCheck. Also change the method name binaryTreeBST to isBST or isBinarySearchTree to reflect what the method does.

Code Snippets

12
   /  \
  9    15
 / \
3   5

Context

StackExchange Code Review Q#15785, answer score: 2

Revisions (0)

No revisions yet.