patternjavaMinor
Binary tree max sum level - better design?
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:
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.
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.