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

Binary tree max sum level - better design?

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

Problem

I have written some code for finding a level in a binary tree, having a maximum number of elements. I have a few questions:

  • Is it a good design? I have used 2 queues but the total sum of elements both queues store will be less than n. So I think it should be OK.



  • Can there be a better design?



public class MaxSumLevel {  
    public static int findLevel(BinaryTreeNode root) {      
        Queue mainQ = new Queue();
        Queue tempQ = new Queue();
        int maxlevel = 0;
        int maxVal = 0;
        int tempSum = 0;
        int tempLevel = 0;
        if (root != null) {
            mainQ.enqueue(root);
            maxlevel = 1;
            tempLevel = 1;
            maxVal = root.getData();
        }           
        while ( !mainQ.isEmpty()) {         
            BinaryTreeNode head = (BinaryTreeNode) mainQ.dequeue();
            BinaryTreeNode left = head.getLeft();
            BinaryTreeNode right = head.getRight();
            if (left != null) {
                tempQ.enqueue(left);
                tempSum = tempSum + left.getData();
            }
            if (right != null) {
                tempQ.enqueue(right);
                tempSum = tempSum + right.getData();
            }           
            if (mainQ.isEmpty()) {
                mainQ = tempQ;
                tempQ = new Queue();
                tempLevel ++;
                if (tempSum > maxVal) {
                    maxVal = tempSum;
                    maxlevel = tempLevel;
                    tempSum = 0;
                }               
            }           
        }
        return maxlevel;
    }
}

Solution

My initial criticism is that your method doesn't have javadoc comments that state clearly and unambiguously what the method is supposed to do.

Why am I saying that?

Well, mainly because your Question has exactly the same problem! You say:


I have written some code for finding a level in a binary tree, having a maximum number of elements.

This is bad description of what you are (apparently) trying to do:

  • The word "level" usually refers to the distance of a node from (say) the root of a tree. But you apparently mean the "height" of the entire tree.



  • The phrase "having a maximum number of elements" is unclear. What are you talking about? How does this affect the problem? (I don't see a maximum number of elements parameter ... or any reference to this in your code.)



So why does this matter?

Because I (the reader) should not have to read through your code to try to figure out the problem you are trying to solve. Especially if your implementation is non-obvious ... which it is ... and possibly contains mistakes that might lead me to think it is solving a different problem than you are actually trying to solve.

Context

StackExchange Code Review Q#14603, answer score: 2

Revisions (0)

No revisions yet.