Skip to main content
added check
Source Link
dfhwze
  • 14.2k
  • 3
  • 40
  • 101

Reading Material

.. any reading material about searching algorithms on trees

These are the most common tree walkers:


Review

There is a bug with IsRoot. Also, why not provide a property Root { get; }?

if the parent is the root of the tree, then the parent is set to null

public bool IsRoot { get { return Parent != null; } }

You should also take advantage of the sweet syntactic sugar of the language (for all your properties):

 public bool IsRoot => Parent == null;

Since Children is private and you always instantiate a list, there is no reason to use null-propagation here:

public int Count { get { return Children?.Count ?? 0; } }

public int Count => Children.Count;

AddChild should throw exceptions on invalid input. You don't check for an invalid tree, what if the node is a grand-parent of the the current instance? Perform similar checks for RemoveChild.

public void AddChild(Node node)
{
    node = node ?? throw new ArgumentNullException(nameof(node));
    if (IsAncestorOrSelf(node)) // <- you should implement such method
        throw new ArgumentException("The node can not be an ancestor or self");
    if (IsDescendant(node)) // <- you should implement such method
        throw new ArgumentException("The node can not be a descendant");
    node.Parent = this;
    Children.Add(node);
}

GetChildren should return an immutable copy of the list containing the children.

public IEnumerable<Node> GetChildren()
{
    return Children.ToArray();
}

I don't know why you would need DeepCopy functionality.


GetChildrenRecursive should be called GetDescendants. I would implement it using recursion. This is implemented as depth-first (DFS).

public IEnumerable<Node> GetDescendants()
{
    foreach (var child in Children)
    {
         yield return child;
         foreach (var descendant in child.GetDescendants())
         {
              yield return descendant;
         }
    }
}

Reading Material

.. any reading material about searching algorithms on trees

These are the most common tree walkers:


Review

There is a bug with IsRoot. Also, why not provide a property Root { get; }?

if the parent is the root of the tree, then the parent is set to null

public bool IsRoot { get { return Parent != null; } }

You should also take advantage of the sweet syntactic sugar of the language (for all your properties):

 public bool IsRoot => Parent == null;

Since Children is private and you always instantiate a list, there is no reason to use null-propagation here:

public int Count { get { return Children?.Count ?? 0; } }

public int Count => Children.Count;

AddChild should throw exceptions on invalid input. You don't check for an invalid tree, what if the node is a grand-parent of the the current instance? Perform similar checks for RemoveChild.

public void AddChild(Node node)
{
    node = node ?? throw new ArgumentNullException(nameof(node));
    if (IsAncestorOrSelf(node)) // <- you should implement such method
        throw new ArgumentException("The node can not be an ancestor or self");
    node.Parent = this;
    Children.Add(node);
}

GetChildren should return an immutable copy of the list containing the children.

public IEnumerable<Node> GetChildren()
{
    return Children.ToArray();
}

I don't know why you would need DeepCopy functionality.


GetChildrenRecursive should be called GetDescendants. I would implement it using recursion. This is implemented as depth-first (DFS).

public IEnumerable<Node> GetDescendants()
{
    foreach (var child in Children)
    {
         yield return child;
         foreach (var descendant in child.GetDescendants())
         {
              yield return descendant;
         }
    }
}

Reading Material

.. any reading material about searching algorithms on trees

These are the most common tree walkers:


Review

There is a bug with IsRoot. Also, why not provide a property Root { get; }?

if the parent is the root of the tree, then the parent is set to null

public bool IsRoot { get { return Parent != null; } }

You should also take advantage of the sweet syntactic sugar of the language (for all your properties):

 public bool IsRoot => Parent == null;

Since Children is private and you always instantiate a list, there is no reason to use null-propagation here:

public int Count { get { return Children?.Count ?? 0; } }

public int Count => Children.Count;

AddChild should throw exceptions on invalid input. You don't check for an invalid tree, what if the node is a grand-parent of the the current instance? Perform similar checks for RemoveChild.

public void AddChild(Node node)
{
    node = node ?? throw new ArgumentNullException(nameof(node));
    if (IsAncestorOrSelf(node)) // <- you should implement such method
        throw new ArgumentException("The node can not be an ancestor or self");
    if (IsDescendant(node)) // <- you should implement such method
        throw new ArgumentException("The node can not be a descendant");
    node.Parent = this;
    Children.Add(node);
}

GetChildren should return an immutable copy of the list containing the children.

public IEnumerable<Node> GetChildren()
{
    return Children.ToArray();
}

I don't know why you would need DeepCopy functionality.


GetChildrenRecursive should be called GetDescendants. I would implement it using recursion. This is implemented as depth-first (DFS).

public IEnumerable<Node> GetDescendants()
{
    foreach (var child in Children)
    {
         yield return child;
         foreach (var descendant in child.GetDescendants())
         {
              yield return descendant;
         }
    }
}
added 27 characters in body
Source Link
dfhwze
  • 14.2k
  • 3
  • 40
  • 101

Reading Material

.. any reading material about searching algorithms on trees

These are the most common tree walkers:


Review

There is a bug with IsRoot. Also, why not provide a property Root { get; }?

if the parent is the root of the tree, then the parent is set to null

public bool IsRoot { get { return Parent != null; } }

You should also take advantage of the sweet syntactic sugar of the language (for all your properties):

 public bool IsRoot => Parent == null;

Since Children is private and you always instantiate a list, there is no reason to use null-propagation here:

public int Count { get { return Children?.Count ?? 0; } }

public int Count => Children.Count;

AddChild should throw exceptions on invalid input. You don't check for an invalid tree, what if the node is a grand-parent of the the current instance? Perform similar checks for RemoveChild.

public void AddChild(Node node)
{
    node = node ?? throw new ArgumentNullException(nameof(node));
    if (IsAncestorOrSelf(node)) // <- you should implement such method
        throw new ArgumentException("The node can not be an ancestor or self");
    node.Parent = this;
    Children.Add(node);
}

GetChildren should return an immutable copy of the list containing the children.

public IEnumerable<Node> GetChildren()
{
    return Children.ToArray();
}

I don't know why you would need DeepCopy functionality.


GetChildrenRecursive should be called GetDescendants. I would implement it using recursion. This is implemented as depth-first (DFS).

public IEnumerable<Node> GetDescendants()
{
    foreach (var child in Children)
    {
         yield return child;
         foreach (var descendant in child.GetDescendants())
         {
              yield return descendant;
         }
    }
}

.. any reading material about searching algorithms on trees

These are the most common tree walkers:

Review

There is a bug with IsRoot. Also, why not provide a property Root { get; }?

if the parent is the root of the tree, then the parent is set to null

public bool IsRoot { get { return Parent != null; } }

You should also take advantage of the sweet syntactic sugar of the language (for all your properties):

 public bool IsRoot => Parent == null;

Since Children is private and you always instantiate a list, there is no reason to use null-propagation here:

public int Count { get { return Children?.Count ?? 0; } }

public int Count => Children.Count;

AddChild should throw exceptions on invalid input. You don't check for an invalid tree, what if the node is a grand-parent of the the current instance? Perform similar checks for RemoveChild.

public void AddChild(Node node)
{
    node = node ?? throw new ArgumentNullException(nameof(node));
    if (IsAncestorOrSelf(node)) // <- you should implement such method
        throw new ArgumentException("The node can not be an ancestor or self");
    node.Parent = this;
    Children.Add(node);
}

GetChildren should return an immutable copy of the list containing the children.

public IEnumerable<Node> GetChildren()
{
    return Children.ToArray();
}

I don't know why you would need DeepCopy functionality.


GetChildrenRecursive should be called GetDescendants. I would implement it using recursion. This is implemented as depth-first (DFS).

public IEnumerable<Node> GetDescendants()
{
    foreach (var child in Children)
    {
         yield return child;
         foreach (var descendant in child.GetDescendants())
         {
              yield return descendant;
         }
    }
}

.. any reading material about searching algorithms on trees

These are the most common tree walkers:

Reading Material

.. any reading material about searching algorithms on trees

These are the most common tree walkers:


Review

There is a bug with IsRoot. Also, why not provide a property Root { get; }?

if the parent is the root of the tree, then the parent is set to null

public bool IsRoot { get { return Parent != null; } }

You should also take advantage of the sweet syntactic sugar of the language (for all your properties):

 public bool IsRoot => Parent == null;

Since Children is private and you always instantiate a list, there is no reason to use null-propagation here:

public int Count { get { return Children?.Count ?? 0; } }

public int Count => Children.Count;

AddChild should throw exceptions on invalid input. You don't check for an invalid tree, what if the node is a grand-parent of the the current instance? Perform similar checks for RemoveChild.

public void AddChild(Node node)
{
    node = node ?? throw new ArgumentNullException(nameof(node));
    if (IsAncestorOrSelf(node)) // <- you should implement such method
        throw new ArgumentException("The node can not be an ancestor or self");
    node.Parent = this;
    Children.Add(node);
}

GetChildren should return an immutable copy of the list containing the children.

public IEnumerable<Node> GetChildren()
{
    return Children.ToArray();
}

I don't know why you would need DeepCopy functionality.


GetChildrenRecursive should be called GetDescendants. I would implement it using recursion. This is implemented as depth-first (DFS).

public IEnumerable<Node> GetDescendants()
{
    foreach (var child in Children)
    {
         yield return child;
         foreach (var descendant in child.GetDescendants())
         {
              yield return descendant;
         }
    }
}
added 347 characters in body
Source Link
dfhwze
  • 14.2k
  • 3
  • 40
  • 101

Review

There is a bug with IsRoot. Also, why not provide a property Root { get; }?

if the parent is the root of the tree, then the parent is set to null

public bool IsRoot { get { return Parent != null; } }

You should also take advantage of the sweet syntactic sugar of the language (for all your properties):

 public bool IsRoot => Parent == null;

Since Children is private and you always instantiate a list, there is no reason to use null-propagation here:

public int Count { get { return Children?.Count ?? 0; } }

public int Count => Children.Count;

AddChild should throw exceptions on invalid input. You don't check for an invalid tree, what if the node is a grand-parent of the the current instance? Perform similar checks for RemoveChild.

public void AddChild(Node node)
{
    node = node ?? throw new ArgumentNullException(nameof(node));
    if (IsAncestorOrSelf(node)) // <- you should implement such method
        throw new ArgumentException("The node can not be an ancestor or self");
    node.Parent = this;
    Children.Add(node);
}

GetChildren should return an immutable copy of the list containing the children.

public IEnumerable<Node> GetChildren()
{
    return Children.ToArray();
}

I don't know why you would need DeepCopy functionality.


GetChildrenRecursive should be called GetDescendants. I would implement it using recursion. This is implemented as depth-first (DFS).

public IEnumerable<Node> GetDescendants()
{
    foreach (var child in Children)
    {
         yield return child;
         foreach (var descendant in child.GetDescendants())
         {
              yield return descendant;
         }
    }
}

.. any reading material about searching algorithms on trees

These are the most common tree walkers:

Review

There is a bug with IsRoot. Also, why not provide a property Root { get; }?

if the parent is the root of the tree, then the parent is set to null

public bool IsRoot { get { return Parent != null; } }

You should also take advantage of the sweet syntactic sugar of the language (for all your properties):

 public bool IsRoot => Parent == null;

Since Children is private and you always instantiate a list, there is no reason to use null-propagation here:

public int Count { get { return Children?.Count ?? 0; } }

public int Count => Children.Count;

AddChild should throw exceptions on invalid input. You don't check for an invalid tree, what if the node is a grand-parent of the the current instance? Perform similar checks for RemoveChild.

public void AddChild(Node node)
{
    node = node ?? throw new ArgumentNullException(nameof(node));
    if (IsAncestorOrSelf(node)) // <- you should implement such method
        throw new ArgumentException("The node can not be an ancestor or self");
    node.Parent = this;
    Children.Add(node);
}

GetChildren should return an immutable copy of the list containing the children.

public IEnumerable<Node> GetChildren()
{
    return Children.ToArray();
}

I don't know why you would need DeepCopy functionality.


GetChildrenRecursive should be called GetDescendants. I would implement it using recursion.

public IEnumerable<Node> GetDescendants()
{
    foreach (var child in Children)
    {
         yield return child;
         foreach (var descendant in child.GetDescendants())
         {
              yield return descendant;
         }
    }
}

Review

There is a bug with IsRoot. Also, why not provide a property Root { get; }?

if the parent is the root of the tree, then the parent is set to null

public bool IsRoot { get { return Parent != null; } }

You should also take advantage of the sweet syntactic sugar of the language (for all your properties):

 public bool IsRoot => Parent == null;

Since Children is private and you always instantiate a list, there is no reason to use null-propagation here:

public int Count { get { return Children?.Count ?? 0; } }

public int Count => Children.Count;

AddChild should throw exceptions on invalid input. You don't check for an invalid tree, what if the node is a grand-parent of the the current instance? Perform similar checks for RemoveChild.

public void AddChild(Node node)
{
    node = node ?? throw new ArgumentNullException(nameof(node));
    if (IsAncestorOrSelf(node)) // <- you should implement such method
        throw new ArgumentException("The node can not be an ancestor or self");
    node.Parent = this;
    Children.Add(node);
}

GetChildren should return an immutable copy of the list containing the children.

public IEnumerable<Node> GetChildren()
{
    return Children.ToArray();
}

I don't know why you would need DeepCopy functionality.


GetChildrenRecursive should be called GetDescendants. I would implement it using recursion. This is implemented as depth-first (DFS).

public IEnumerable<Node> GetDescendants()
{
    foreach (var child in Children)
    {
         yield return child;
         foreach (var descendant in child.GetDescendants())
         {
              yield return descendant;
         }
    }
}

.. any reading material about searching algorithms on trees

These are the most common tree walkers:

Source Link
dfhwze
  • 14.2k
  • 3
  • 40
  • 101
Loading