CODEWITHSUNDEEP
c#
coder
coder

Reputation: 4283

c# parent child design

I am designing a c# class and would like to know if my design is right.

abstract class PersonBase
{
    public string Name { get; set; }
    public PersonBase Parent { get; set; }
    public List<PersonBase> Children { get; set; }

}

class Person : PersonBase
{
    //public override List<Person> Children { get; set; }

    public Person()
    {
        Children = new List<PersonBase>();
    }

    public void Add(Person child)
    {
        child.Parent = this;
        Children.Add(child);
    }
}

test code:

private void button1_Click(object sender, EventArgs e)
{
    Person parent = new Person();
    parent.Name = "parent";

    Person child = new Person();
    child.Name = "child1";
    child.Add(new Person() { Name = "grandchild1" });

    parent.Add(child);

}

It works as expected. I am able to access the parent children objects from anywhere in the hierarchy. My concern is it looks recursive or circular reference (can't find the right word here).

Here is what I did finally:

public class Person
{
    public string Name { get; set; }
    public Person Parent { get; set; }
    public List<Person> Children { get; private set; }

    public Person()
    {
        Children = new List<Person>();
    }

    public void AddChild(Person child)
    {
        if (child == this) { return; }
        if (Children.Contains(child)) { return; }

        child.Parent = this;
        Children.Add(child);
    }
}

Upvotes: 0

Views: 4933

Answers (4)

Cubicle.Jockey
Cubicle.Jockey

Reputation: 3328

Like @LukeH asked your break out of BasePerson and Person do not make sense. This will work just fine.

public class Person
{
    public string Name { get; set; }
    public Person Parent { get; set; }
    public IList<Person> Children { get; private set; }

    public Person()
    {
        Children = new List<Person>();
    }

    public void Add(Person child)
    {
        child.Parent = this;
        Children.Add(child);
    }
}

If you are going to have different kinds of Person then you might want to break you stuff out into an Interface if there is not inherited logic and an abstract class if you want to provide some default logic.

EDIT: Adding expressed issues from Danny

Upvotes: 2

Kyle W
Kyle W

Reputation: 3752

//Declare and initialize Person a, b, c;
a.Add(c);
b.Add(c); // Now a thinks c is a child, but c does not think a is a parent!

You need some sort of validation, perhaps that c doesn't already have a parent, or set the parent/child only in a Parent.CreateChild method. Or allow it to have multiple parents.

(Also, I would declare the method AddChild, since that's what it's doing. And also pay attention to the design considerations from other commenters.)

Perhaps this:

class Person
{
    public string Name { get; private set; }
    public Person Parent { get; private set; }
    public IList<Person> Children { get; private set; }

    private Person() {} // Private constructor

    public static Person CreatePersonNoParent(string name){*implementation elided*};
    public Person CreateChild(string name)
    {
        Person child = new Person { Name=name, Parent=this };
        this.Children.Add(child);
        return child;
    }
}

Upvotes: 1

Danny Varod
Danny Varod

Reputation: 18068

The base class seams redundant, also, you are assuming children is not null and assigning in constructor, yet it has a public setter.

Change children's setter to private or protected and don't worry about circular reference - just mark parent property with an attribute to prevent it from being serialized.

Upvotes: 2

M3NTA7
M3NTA7

Reputation: 1347

It could become a problem if you used child.Add(Me or MyParent or ancestor). Then it would be an endless loop of references. You might want to add code in the Add method to prevent improper usage so a 'Person' can not add itself or it's parents as a child.

Upvotes: 2

Related Questions