onsdag 19. mai 2010

Focus on functionality, use technology - Revisited

Lars-Erik Kindblad  posted some great questions about the implementation in my previous post. I thought I'd post the reply here as the questions pins a lot of the thought behind the previous post. The code up for discussion is:

public void BlockCustomer(int customerID)
{
    var customer = getCustomer(customerID);
    customer.Block();
}

He's questions were:
1) How would you go about unit testing the second example? Since the customer is not injected into the constructor, the unit test would have a dependency on the customer.Block() method?
2) Would customer.Block() (and customer.Save(), Remove(), Notify() etc.) break the Single Responsibility Principle?
3) I agree customer.Block() creates more readable code, but it can create large and messy entity classes when the code base is growing. I've seen such classes with 1000+ lines of code. In that case I rather prefer using manager/service classes in order to partition the functionality across multiple classes.
So, first thing. In my code I use repositories extensively. Most likely the getCustomer method would use some repository to fetch the customer. The class holding the BlockCustomer method would therefore have the ICustomerRepository injected. When dealing with dependency injection I separate between stateless classes and state full classes. Usually I automatically inject stateless classes through a dependency injection container.  The repository class is a stateless class and would be injected into the class holding the BlockCustomer method automatically. The repository would be responsible for constructing the state full class Customer. To make sure that we are  able to write maintainable tests for the BlockCustomer method we'll have the repository return an ICustomer interface.

On to the second question, what about SRP. He is absolutely right, having the save and remove methods on the customer would break the Single Responsibility Principle.When using a domain model I'm not a big fan of using that type of active record implementation. Add and Remove is something I would have implemented in the ICustomerRepository. Within the repository I would use some kind of data mapper framework (NHibernate, EF..). Preferably one that supports automatic entity change tracking so that I would not have to implement Save. But still he has a point. From the first example in the previous post we could see that customer.Block() would have to do more than modifying it's own state. However I don't see anything wrong with it delegating the rest of it's work to a dependency. The dependency would be injected into the Customer class by the customer repository before handing it off.

class Customer
{
    ...
    private CustomerState _state;
    private IDependencyNeededToBlock _dependencyNeededToBlock;
    
    public IDependencyNeededToBlock DependencySetter
    {
        set
        {
            if (_dependencyNeededToBlock != null)
                throw new Exception("Dependency already set");
            _dependencyNeededToBlock = value;
        }
    }

    public void Block()
    {
        _state = CustomerState.Blocked;
        _dependencyNeededToBlock.DoWhatIsNeeded();
    }
}

However what is important to note is that the implementation should show the intention of the functionality. Since the intent of this functionality was blocking a customer it's natural that the functionality is initiated by the customer. If on the other hand the functionality was about something else and part of it was blocking the customer then of course that other functionality would not be delegated by the customer.

If the Customer class now is limited to modifying internal state and calling dependencies we should have a nice and maintainable class. On the other hand if it still ends up as a 1000+ lines class we probably need to redefine our perception of the word customer :)

Ingen kommentarer:

Legg inn en kommentar