chewbyte
chewbyte

Reputation: 25

What's wrong with this tree implementation in C#?

I'm currently trying to implement a very simple tree/node class in C#, where nodes have an object as data, and they can have zero to many children. I'm currently having two problems though:

  1. For some reason, printing out the object ends up printing the TYPE of the object instead of it's toString() for every node that isn't the root.
  2. I can't seem to print out multiple branches of my tree correctly, and can't locate the issue, whether it be a problem with my printing method or the way I am adding children to nodes.

My Node class is below.

namespace Tree
{
    class Node
    {
        public object data;
        private LinkedList<Node> children;

        public Node(object data)
        {
            this.data = data;
            children = new LinkedList<Node>();
        }

        public void Add(params object[] objects)
        {
            foreach (object obj in objects)
            {
                children.AddLast(new Node(obj));
            }
        }

        public int Count()
        {
            int count = 1;

            foreach (Node n in children)
            {
                count += n.Count();
            }

            return count;
        }

        public void Print(int depth)
        {
            string s = new string('\t',depth);
            s += data;
            Console.WriteLine(s);
            depth++;

            foreach (Node n in children)
            {
                n.Print(depth);
            }
        }
    }
}

To test, I am creating a tree with a root with three children, and each of those three children then have three further children, as below.

Node core = new Node("root");

Node o1 = new Node("1");
Node o2 = new Node("2");
Node o3 = new Node("3");

o1.Add(new Node("11"), new Node("12"), new Node("13"));
o2.Add(new Node("21"), new Node("22"), new Node("23"));
o3.Add(new Node("31"), new Node("32"), new Node("33"));
core.Add(o1, o2, o3);

Console.WriteLine(core.Count());
core.Print(0);

Expected output would of course be:

13
root
 1
  11
  12
  13
 2
  21
  22
  23
 3
  31
  32
  33

Unfortunately I get:

4
root
    Tree.Node
    Tree.Node
    Tree.Node

This is my first time doing recursion in C#, so maybe there's something simple I'm missing. If this is the case I would rather have the problem explained rather than the solution given to me in code. Thank you.

Upvotes: 1

Views: 107

Answers (5)

Enigmativity
Enigmativity

Reputation: 117175

This isn't directly an answer to your question, just more of a suggestion as to how to structure your class.

Try this:

public class Node<T> : LinkedList<Node<T>>
{
    public T Data { get; set; }

    public Node(T data)
    {
        this.Data = data;
    }
}

That's it. Well, at least that's it for your core code. You need this set of extension methods to work with it:

public static class NodeEx
{
    public static void Add<T>(this Node<T> tree, Node<T> child)
    {
        tree.AddLast(child);
    }

    public static int Count<T>(this Node<T> tree)
    {
        int count = 1;
        foreach (Node<T> n in tree)
        {
            count += n.Count();
        }
        return count;
    }

    public static void Print<T>(this Node<T> tree, int depth)
    {
        Console.WriteLine(new string('\t', depth) + tree.Data);
        foreach (Node<T> n in tree)
        {
            n.Print(depth + 1);
        }
    }
}

Now with the void Add<T>(this Node<T> tree, Node<T> child) extension method you can write this code:

Node<string> core = new Node<string>("root")
{
    new Node<string>("1")
    {
        new Node<string>("11"),
        new Node<string>("12"),
        new Node<string>("13")
    },
    new Node<string>("2")
    {
        new Node<string>("21"),
        new Node<string>("22"),
        new Node<string>("23")
    },
    new Node<string>("3")
    {
        new Node<string>("31"),
        new Node<string>("32"),
        new Node<string>("33")
    },
};

The int Count<T>(this Node<T> tree) & void Print<T>(this Node<T> tree, int depth) work as expected. This code:

Console.WriteLine(core.Count());
core.Print(0);

...produces:

13
root
  1
    11
    12
    13
  2
    21
    22
    23
  3
    31
    32
33

Now, the big advantage is that all of the normal methods available against the LinkedList<T> object works for Node<T>.

Upvotes: 0

Alex B.
Alex B.

Reputation: 2167

Additionally to the existing answers, don´t concat strings with +=:

s += data;

Use

s = String.Concat(s, data.ToString()); 

instead.

Also does the data really need to be of type object? Wihtout knowing your whole system this is just a guess, but it could be feasible to have a generic Nodeclass like:

class Node<T> 
{
  public T data;
  ...
  public void AddChild(Node<T> childNode) ...
  public void AddChilds(IEnumerable<Node<T>> childNode) ...
}


Node<String> root = new Node<String>("root");
root.AddChild(new Node<String>("FirstBelowRoot");

Upvotes: 0

user932887
user932887

Reputation:

public Node(object data)
{
    this.data = data;
    children = new LinkedList<Node>();
}

Here you're initializing this node's data to an object of type Node, when you do something like Add(new Node("11")). Essentially, your constructed node now contains another node as data, instead of "11", as you initially intended.

Don't use object for anything, there should be no reason to do it as part of your C# learning, and it can only bite you as you found out here. Use either generics or tagged unions for types to obtain nodes that can contain data of different types.

Learn about generics then revisit your tree implementation, is my advice.

Upvotes: 0

NineBerry
NineBerry

Reputation: 28549

The problem is with your Add() method. Currently it is implemented to receive objects and add nodes with these objects. But you are using it to add child nodes. You would need two different methods:

public void AddObjects(params object[] objects)
{
    foreach (object obj in objects)
    {
        children.AddLast(new Node(obj));
    }
}

public void AddChildNodes(params Node[] nodes)
{
    foreach (Node node in nodes)
    {
        children.AddLast(node);
    }
}

Then where you setup the tree structure, use AddChildNodes() instead of Add()

This is how the setup code could then look:

Node core = new Node("root");

Node o1 = new Node("1");
Node o2 = new Node("2");
Node o3 = new Node("3");

o1.AddObjects("11", "12", "13");
o2.AddObjects("21", "22", "23");
o3.AddObjects("31", "32", "33");
core.AddChildNodes(o1, o2, o3);

Console.WriteLine(core.Count());
core.Print(0);

Upvotes: 1

M. Buga
M. Buga

Reputation: 494

Quick fix:

public void Add(params Node[] objects)
{
    foreach (Node obj in objects)
    {
        children.AddLast(obj);
    }
}

If your Add method supposed to add child nodes then first you should use corresponding type for objects argument. Second you should remove additional object conversion to Node because you already passing Node type arguments.

Upvotes: 2

Related Questions