Added root parameter

[xam4lor]

Added root parameter instead of having a global variable. @see #134.

[zsparal]

I don't really think this is the way to fix this problem, it's not very idiomatic C# code. You could just get rid of the Node class and move its two properties to Tree (and make them private). That way you don't have to create dummy nodes in advance.

[xam4lor]

May be do that for the full example but I think that It's more comprehensive for the website which shows functions 1 by 1.

[zsparal]

I disagree, every language should implement the algorithms in a way that's idiomatic for that specific language. For example you won't see parallels between the C and Haskell implementations (and you really shouldn't).

[xam4lor]

The problem is that if we do what you said, on the website a method like CreateTree(int depthCount, int childrenCount) is going to use a parameter called root but the user cannot know what mean this parameter, where it was instanciate, ...
But I agree with you that it's not the better way for an object-oriented language but this will do the job, at least for the functions isolated on the website !

[zsparal]

I still disagree. If anyone wonders where root comes from they can just check the full code example. The main selling point of OO languages is encapsulation, we should follow best practices regardless of the format of the book. Also people who are at least somewhat competent in C# will probably guess where that ariable comes from. If you really want you can use it as this.root if you want to make it really obvious.

[xam4lor]

I think that the users reading this book should understand how it works directly without having to check the full code ! Using this. is really obvious so why not create a comment just above the first use of the parameter root ?

[june128]

@Gustorn We could maybe move Node in a separate file, make an interface for it and let the algorithm use the interface; that way it can get used by all Node classes using the interface.
This is the only way I can think of on how to implement the functionality of having a root parameter.
The question is tho, if we want to make the algorithm really that complex (but then also usable).

[zsparal]

The thing is you don't want the root parameter. It makes no sense to have it (and as I said, you don't really need the Node class either).

[xam4lor]

If think it makes more sense to have a node parameter than a root node variable.

[zsparal]

You don't need a root node variable.

public class Tree {
    public int Value { get; set; }
    private List<Tree> _children;
}

[june128]

@Gustorn you're right, your solution is maybe even better. But I would name the class still Node instead of Tree.

[june128]

Aside from that, with a root parameter the classes would've to be split up into Tree and TreeTraversal of course. But I'm not really sure, that's a good way to do it. Leaving it as is or doing your suggestion seems easier and more straightforward.

[june128]

It's after all (like Buttercak3 mentioned in #141) a learning book & not a library.

[zsparal]

I'll obviously vote for my solution :) My argument for keeping the name Tree: it's a recursive datastructure anyway, so it makes sense to call it Tree. It also reflects that no matter which child/node/etc. you take you still have a valid tree.

But that particular detail is pretty inconsequential, I'm fine with either Node or Tree.

[xam4lor]

I have an other solution which is made just of one 'code' (basically two file but that's because of the language) in Java for this example. The root parameter is a variable in the class himself but is also passed as a parameter to a specific function. Look at the issue #149 and tell me what do you think of that !

[june128]

@xam4lor Ok, but why should we have the parameter, if the root is already in the class & public? That doesn't make much sense to me. Either one or the other I would say.
What do you think @Gustorn ?

[june128]

@xam4lor @Gustorn suggested to change the style of the algorithm and apply what we discussed here in the C# PR I have currently open. So let's move the discussion there. #120