Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

Here is my code.

/**
 * Definition for a binary tree node.
 * public class TreeNode {
 *     int val;
 *     TreeNode left;
 *     TreeNode right;
 *     TreeNode(int x) { val = x; }
 * }
 */
public class Solution {
 public List<List<Integer>> levelOrder(TreeNode root) {
        List<List<Integer>> list = new LinkedList<List<Integer>>();
        if (root == null) {
            return list;
        } else {
            Queue<TreeNode> q = new LinkedList<>();
            q.add(root);
            while (!q.isEmpty()) {
                int size = q.size();
                LinkedList<Integer> layers = new LinkedList<>();
                while (size != 0) {
                    TreeNode current = q.poll();
                    layers.add(current.val);
                    if (current.left != null) {
                        q.add(current.left);
                    }
                    if (current.right != null) {
                        q.add(current.right);
                    }
                    size--;
                }

                list.add(layers);

            }
        }
        return list;

    }

}

Please let me know if any improvement/correction.

share|improve this question
    
Some vertical space (new lines) to group related code would increase readability. – Heslacher 2 days ago

Nicely done. I especially like how you used the size of the queue to know how many items are there on the level. I have only minor improvement ideas.


You can simplify this declaration:

List<List<Integer>> list = new LinkedList<List<Integer>>();

Using the diamond operator <>, like you already did at other places:

List<List<Integer>> list = new LinkedList<>();

The code is deeply indented. You could reduce the indent level by eliminating the else branch of the outermost condition:

    if (root == null) {
        return list;
    }
    Queue<TreeNode> q = new LinkedList<>();
    q.add(root);
    // ...

The while loop could be written as a for loop. The benefit is that it will be harder to forget to decrement size.


Lastly, some of the names could be better.

  • Instead of "layers", I propose singular "layer"
  • Instead of "size", I propose "count"
share|improve this answer
    
Thanks Janos for detailed explanation. – Mosbius8 2 days ago

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.