+972 52-548-6969
Who is the responsible party?
Object Oriented design is all about placing behavior and data together, of making the object responsible for its own operations. The problem then becomes identifying which object is responsible for what. This is usually pretty easy, since the natural owner of an operation is usually clear. Note that I don’t much care for the choice people make between things like parent.AddChild(child) vs. child.AddTo(parent). Both are equally good from my point of view. What I don’t want to see is this:
parent.Children.Add(child);
child.Parent = parent;
The reason for that is quite simple. This is a serious violation of the Tell, Don’t Ask protocol, and it means that someone is missing the pointing between the ownership of a particular operation and the instigator of that operation. The instigator is usually the UI, requesting that something will happen. Bad code will usually put a lot of the logic in the instigator side, rather than on the owner side.
Then we move into issues of cohesiveness and coupling, which guides us further in understanding how to place the appropriate responsibilities. All logic related to a particular aspect of the application should reside in the same location, so it is easy to work with, modify and change.
But by far the most common error I see people make is placing responsibilities on the instigator rather than on other parts of the system. There should be a very clear distinction between the two. Other than that, there is a lot of stuff that is just experience and understanding of the actual system.
In the end, all systems are refactorable, so you shouldn’t be afraid to make mistakes.
Comments
There are times you have to set a children's parent, such as when using a foreign generator in NHib with 1 to 1 relationships and you want to propagate save-update.
Scott,
The question is, who is responsible for doing so
This is one of the big reasons why I like NHiberate, the parent can easily hide the underlying children collection and dictate how you get at and set its children.
Most other ORMs that I have been exposed to (not naming names to avoid a flame war) makes the children collection public on the parent and does not offer a way of controlling access to the collection.
It is possible (though not necessarily easier) to place logic in collection classes and properties so that
parent.Children.Add(child); // Children is a custom collection
child.Parent = parent; // Parent is a property, not a field
does not violate Tell, Don't Ask because the Parent and ChildCollection classes (and any classes derived from them) implement all the logic. .NET does this repeatedly, I think.
Bidi collections tend to cause a lot of problems in the long run
So then how does that explain for instance the reason for some people creating an ObjectTask class which would do some additional functionality related to the behaviour(e.g CartTask.AddToCart(Cart cart, Item someItem) where CartTask would calculate and set shipping or log history or any additional tasks) shouldnt these be handled by the AddtoCart function?
What about the following?
parent.AddChild(child);
where::
public virtual void AddChild(Person child)
{
child.Parent = this;
this.children.Add(child);
}
@Scott. Why create (and introduce) a new method? Are you also adding a Get and Delete method? If so, you're violating the S in the SOLID principle where a class should have only one (single) responsibility.
I guess you already have a collection of children. So why not override the Add itself.
public class ChildCollection : List <child
{
private Parent parent;
public ChildCollection(Parent parent)
{
}
public override void Add(Child item)
{
}
}
public class Parent
{
private readonly ChildCollection childs;
public ChildCollection Childs { get { return childs; } }
public Parent()
{
}
}
That prevents that methods in your class (or a derived one) can bypass AddChild and add a child directly (without setting the parent). It also means you can make Childs a public property.
Just my 2 eurocents..
Tariq,
That is call Command pattern, and it is fairly common way to deal with externalizing the logic while keeping it close.
I wouldn't have thought this was a tell-don't-ask scenario. I'd have said it was more a connascence of algorithm.
@Dave
I would have other validation logic in there. To create a bad example, what if a parent is not allowed to have 0 children? I would have a remove method on the Parent class because it's a business rule that says a Parent must have at least one child.
I understand what you're saying though, and I like your code sample. Just trying to bring up an alternative form.
Comment preview