Can someone code review my "Distance between nodes in a BT"?

  • Thread starter Thread starter SlurrerOfSpeech
  • Start date Start date
  • Tags Tags
    Code Nodes Review
AI Thread Summary
The discussion centers around a coding solution for finding the distance between nodes in a binary search tree, which was submitted for an online test but ultimately led to rejection. Key points include the implementation of a binary tree with methods for adding nodes, finding the nearest common ancestor, and calculating distances between nodes. Feedback emphasizes the need for improved code documentation, including comprehensive comments explaining the purpose of the class and methods, as well as clearer variable names. The importance of writing maintainable code that can be easily understood by other programmers is highlighted, suggesting that the current implementation lacks clarity and organization necessary for professional environments. Additionally, there is a suggestion to format comments for compatibility with documentation generators like Doxygen.
SlurrerOfSpeech
Messages
141
Reaction score
11
Here's the solution I used for an online coding test with a major employer and I got rejected. Let me know about improvements I could make. Thanks.

Code:
using System;
using System.Collections.Generic;
using System.Linq;

class Node
{
    public int Val { get; set; }
    public Node Right { get; set; }   
    public Node Left { get; set; }
    public Node(int val) { Val = val; }
}public class Solution
{ 
    static Node _root = null;
   
    static void AddToTree(int val)
    {
        if(_root == null)
            _root = new Node(val);   
       
        for(Node cur = _root; ; )
        {   
            if(cur.Val < val)
            {
                if(cur.Right == null)
                {
                    cur.Right = new Node(val);
                    break;
                }
                else
                {
                    cur = cur.Right;
                }
            }
            else if(val < cur.Val)
            {
                if(cur.Left == null)
                {
                    cur.Left = new Node(val);
                    break;
                }
                else
                {
                    cur = cur.Left;
                }
            }
            else
            {
                break;
            }
        }
    }
   
   
    static Node NearestCommonAncestor(int i, int j)
    {
        int smaller = i < j ? i : j, 
           larger = i + j - smaller;
        return NearestCommonAncestor(smaller, larger, _root);
    }
   
    static Node NearestCommonAncestor(int i, int j, Node root)
    {
        if(root == null)
            return null;
        else if(root.Val < i)
            return NearestCommonAncestor(i, j, root.Right);
        else if(root.Val > j)
            return NearestCommonAncestor(i, j, root.Left);
        else
            return root;
    }

    static int DistanceToNode(Node start, int val)
    {
        int d = 0;
        for(Node cur = start; ; )
        {
            if(cur == null)
                return -1;
            else if(cur.Val < val)
            {
                cur = cur.Right;
                d += 1;
            }
            else if(cur.Val > val)
            {
                cur = cur.Left;
                d += 1;
            }
            else
                break;
        }
        return d;
    }
   
    static int DistanceBetweenNodes(int i, int j)
    {
        Node nca = NearestCommonAncestor(i, j);
        if(nca == null)
            return -1;
        int di = DistanceToNode(nca, i);
        int dj = DistanceToNode(nca, j);
        if(di == -1 || dj == -1)
            return -1;
        return di + dj;
    }
   
    public static void Main(String[] args)
    {
        int[] vals = { 2, 1, 5, 3, 8 };
        foreach(int i in vals)
            AddToTree(i);
        /*
                Now the tree should look like 
               
                2
              / \
             1   5
                / \
                3   8
                             
        */
        IEnumerable<string> results = 
            from x in vals
            from y in vals
            select string.Format
            (
                "Distance between {0} and {1} is {2}",
                x, y, DistanceBetweenNodes(x,y)
            );
        foreach(string message in results)
            Console.WriteLine(message);
    }
}
 
Technology news on Phys.org
Ah, I just realized that I could get rid of the

Code:
        if(nca == null)
            return -1;

line and it would run the same.
 
You might want to tell us what this code is supposed to do.
 
Here's my code review:
Without really studying your code, it seems like the overall organization and algorithm may be ok. That being said, it is not nearly ready for code review.
You need MANY more comments. At the top, give a top level summary of what the class is for. Each method needs a description of what it does, definitions of the inputs and outputs, any special comments about algorithms used. The class name "Solution" is totally inadequate for identifying what this stuff is. Anything tricky should have a comment about what is going on. Many variable names should be more descriptive of what they are.
I hate being so harsh, but I would not accept this code if someone gave it to me. I wouldn't even try to figure it out.

PS. Keep in mind that a company will have programs with millions of lines of code. And a programmer will spend most of his time diving into the middle of code that he did not program and doesn't have the time to figure out the whole thing with no help. Try to make your code such that another programmer can do that. You will be on the receiving end of code like that.

PPS. It is good practice to format your comments so that a documentation generator like Doxygen can use the top level ones you want it to use and will ignore the lower level detail ones.
 
Last edited:
Dear Peeps I have posted a few questions about programing on this sectio of the PF forum. I want to ask you veterans how you folks learn program in assembly and about computer architecture for the x86 family. In addition to finish learning C, I am also reading the book From bits to Gates to C and Beyond. In the book, it uses the mini LC3 assembly language. I also have books on assembly programming and computer architecture. The few famous ones i have are Computer Organization and...
I have a quick questions. I am going through a book on C programming on my own. Afterwards, I plan to go through something call data structures and algorithms on my own also in C. I also need to learn C++, Matlab and for personal interest Haskell. For the two topic of data structures and algorithms, I understand there are standard ones across all programming languages. After learning it through C, what would be the biggest issue when trying to implement the same data...
Back
Top