C#: Add child to collection and set child's parent in same call

Question

I'm building a sort of domain layer and I would like to be able to implement the following methods:

[Note: ISet is a collection class which doesn't permit duplicates, according to checking using .Equals().]

public class Parent
{
public ISet Children = new HashedSet<Child>;
public void AddChild()
{
...
}
public void RemoveChild()
{
...
}
}

public class Child
{
public Parent Parent
{
get {...} set {...}
}
}

Such that the following tests would pass:

When parent.AddChild(child) is called:

• parent.Children contains child.
• child.Parent is set to parent.
• if child had a previous Parent, then that Parent's Children collection no longer contains child

When calling parent.RemoveChild(child):

• child.Parent == null.
• parent.Children doesn't contain child.

And that setting child.Parent is equivalent to calling parent.AddChild(). And that setting child.Parent = null when the child has an existing parent is equivalent to calling parent.RemoveChild().

But I really can't seem to do it!

This must be very common logic in domain layers using e.g. NHibernate, so I was wondering how other people do it?

Answer 1

Use a property and variable on child like this

private Parent _parent;
public Parent parent { 
    get { return _parent; }
    set {
        if(_parent == value)
             return;  //Prevents circular assignement
        if(null != _parent) {
            Parent temp = _parent;
            _parent = null;
            temp.removeChild(this);
        }
        if(null != value) {
             _parent = value;
             value.addChild(this);  
        }
    }
}

This will cause that setting the childs parent property will call the parent methods and all other logic can be put in those methods.

Parent should of cause check if childs parent property needs changing but the set methods also bails out if trying to set same parent again. AR

Answer 2

I would derive the Parent class from the ISet and override the Add and Remove methods to do the job of adding the child to the list and setting its parent.

Simply you can write the logic of adding the child to the parent collection in the set accessor of the Parent property.

Answer 3

There might be a more clever way of doing it, but ultimately you just need to make sure each way of setting the parent/child relationship checks the others, (ie adding a child checks to see if the parent property is already set and setting the parent property checks to make sure the parent contains the child).

// in Parent
public void AddChild(Child c)
{
    // need to check if Parent hasn't yet been set to this
    if (!c.Parent.Equals(this)) c.Parent = this;
    ...
}
public void RemoveChild(Child c)
{
    // need to check if Parent is still set to this
    if (c.Parent.Equals(this)) c.Parent = null;
    ...
}
public bool Contains(Child c)
{
    // assuming ISet implements this function
    return Children.Contains(c);
}

// in Child
public Parent Parent
{
    ...
    set
    {
        Parent old = _Parent;
        _Parent = value;
        if ((old != null) &&
            (old.Contains(this)))
        {
            old.RemoveChild(this);
        }
        if ((_Parent != null) &&
            (!_Parent.Contains(this)))
        {
            _Parent.AddChild(this);
        }
    }
}

I changed the value of _Parent before removing the child since the Child.Parent set code would otherwise be recalled from the Parent.Add/RemoveChild code.

Answer 4

I cannot give a good code answer because I don't understand the semantics of the domain.

• What is a child without parent?
• Why does addchild both adding a parentless child and moving an existing childs
• Are child and parent both aggregateroots
• I guess this is not really about human parents and children, but about some specific domain. Using that in the example can make it possible to provide better answers.

When I knew all those things, I would suggest you to remove all public setters from the domain, and add methods with descriptive name for all domain actions. Users don't manipulate data, but they do things. The method name should be something of your domain semantics and contain all business logic to do that thing.

When parent is an aggregate root for children, I would implement adding a new child to a parent like this:

public class Child
{

  protected Child() { } // Constructor to please NHiberante's "Poco" implementation

  // internal to prevent other assemblies than the domain assembly from constructing childs
  internal Child(string somethingElse, Parent parent)
  {
    SomethingElse = somethingElse;
    Parent = parent;
  }

  // Parent can not be changed by the child itself, because parent is the aggregate root of child.
  public Parent Parent { get; private set; }

  public string SomethingElse { get; private set; }
}

public class Parent
{
  private readonly ISet<Child> children;

  public Parent()
  {
    children = new HashedSet<Child>();
  }

  public IEnumerable<Child> Children
  {
    // only provide read access to the collection because manipulating the collection will disturb the domain semantics.
    get { return children; }
  }

  // This method wouldn't be called add child, but ExecuteSomeBusinessLogic in real code
  public void AddChild(string somethingElse)
  {
    // child constructor can only be called here because the parent is the aggregate root.
    var child = new Child(somethingElse, parent);
    children.Add(child);
  }
}

If you give us some more information, I can give an answer for the other requirements you have.

This answer summarized in a oneliner: "For all state manipulations in your application, you need one method with a name telling what it does."

EDIT: Reaction on comments. I have a problem with one of your specs: only parent is the aggregate root. When that is the case, you would never use the child without the parent. That makes a bidirectional relation useless, because you already know the parent when you access it's child. Bidirectional relations are hard to manage in DDD, so the best solution is to avoid them. I know there are several reasons why you can't avoid them when using NHibernate. The following code has a bidirectional relation and solves the problem of the domain becoming in a temporary invalid state by using internal helpers methods. The internal methods can only be called from the domain assembly, and are not exposed to the outside. I don't really like this approach, but it is the least worse solution I think.

public class Client : AggregateRoot
{

  private readonly ISet<Contact> contacts;

  public Client()
  {
    contacts = new HashedSet<Contact>();
  }

  public IEnumerable<Contact> Contacts
  {
    get { return contacts; }
  }

  public void LogCall(string description)
  {
     var contact = new Contact(description, this);
     AddContact(contact);
  }

  internal void AddContact(Contact contact) 
  { 
    contacts.Add(contact);
  }

  internal void RemoveContact(Contact contact)
  {
    contacts.Remove(contact);
  }
}

public class Contact : AggregateRoot
{
  protected Contact { }

  public Contact(string description, Client client)
  {
    if (description == null) throw new ArgumentNullException("description");
    if (client == null) throw new ArgumentNullException("client");

    Client = client;
    Description = description;
  }

  public Client Client { get; private set; }

  public string Description { get;private set; }

  // I assumed that moving a contact to another client would only be done by the user to correct mistakes?
  // Isn't it an UI problem when the user frequently makes this mistake?
  public void CorrectMistake(Client client)
  {
    Client.RemoveContact(this);
    Client = client;
    client.AddContact(this);
  }
}

Answer 5

I would implement my own ISet, and implement the parent-tracking in there. Use a regular HashSet to do the work of storing the objects, but do some additional work in your class.

public class ChildrenSet : ISet<Child>
{
    private HashSet<Child> backing = new HashSet<Child>();
    private Parent parent;

    internal ChildrenSet(Parent p)
    {
        this.parent = p;
    }

    public bool Add(Child item)
    {
        if(backing.Add(item))
        {
            item.Parent = this.parent;
            return true;
        }
        return false;
    }

    public bool Remove(Child item)
    {
        if(backing.Remove(item))
        {
            item.Parent = null;
            return true;
        }
        return false;
    }

    // etc for the rest of ISet. Also GetEnumerator, if desired.
}

However, this does not implement one thing you mentioned: "if child had a previous Parent, then that Parent's Children collection no longer contains child", since you also said "some entities will have multiple child collections".

When moving the child from one parent to another, if there's multiple collections it could be in, you'd have to check all of those individually. Since it sounds like you're describing a hierarchy of parents, it may be easier to instead throw a InvalidOperationException when attempting to add a child that already has a parent.