Joe Buschmann

let topics = [csharp; specflow; fun]

Lessons from the Past Year

As 2012 comes to a close, it's time to look back on some of the things I've learned. During the past year, my focus has been on enterprise service development, and thanks to a talented software architect and co-worker Kelly Brownsberger, I feel much more confident with my development skills. Together we focused on re-architecting and refactoring large portions of an ordering service. Now, looking back, three lessons in particular have stuck with me and changed the way I develop software.

They are 1) encapsulate operations vertically not horizontally, 2) everything's a service, and 3) separate contract classes from business logic classes. In the remainder of this blog post I expand on these ideas and try to explain why they have made such an impression.

Encapsulate Operations Vertically Not Horizontally

For web service implementations, encapsulation at the operation level (vertically) rather than at the service level (horizontally) seems to work better over the long run. Having an implementation class per operation makes refactoring, unit testing, and reuse much easier.

For example, let's say a hypothetical product ordering service implemented with WCF has a contract IOrderService with several operations and an implementing class OrderService. Most developers would immediately start implementing the service in the OrderService class. Over time OrderService would continue to accumulate implementing code with common code among the operations separated out into private methods or separate classes. Eventually it evolves into a bulky god class. The operations become hard to unit test because it's difficult to know which methods, fields, or private members a particular operation needs. I've seen this happen several times where I work.

[ServiceContract]
public interface IOrderService
{
    [OperationContract]
    InitiateOrderResponse InitiateOrder(
        InitiateOrderRequest request);

    [OperationContract]
    CommitOrderResponse CommitOrder(
        CommitOrderRequest request);

    [OperationContract]
    AddProductToOrderResponse AddProductToOrder(
        AddProductToOrderRequest request);

    [OperationContract]
    RemoveProductFromOrderResponse RemoveProductFromOrder(
        RemoveProductFromOrderRequest request);
}

[ServiceBehavior]
public class OrderService : IOrderService
{
    public InitiateOrderResponse InitiateOrder(
        InitiateOrderRequest request)
    {
        // Lots of implementing code here.
    }

    public CommitOrderResponse CommitOrder(
        CommitOrderRequest request)
    {
        // Lots of implementing code here.
    }

    public AddProductToOrderResponse AddProductToOrder(
        AddProductToOrderRequest request)
    {
        // Lots of implementing code here.
    }

    public RemoveProductFromOrderResponse RemoveProductFromOrder(
        RemoveProductFromOrderRequest request)
    {
        // Lots of implementing code here.
    }
}

As an alternative, this service could be implemented with a class per operation. Each method simply creates an instance of the operation (or retrieves it from a container like Windsor) and executes it. Any common code between the operations can be pulled out into a separate class. For example, they all may return a shopping cart with the latest view of the order. The cart generation code can be encapsulated into the GetShoppingCartTask class and shared among the operations.

This setup is much easier to follow and unit test. Each operation contains just the code it needs to do its job (Single Responsibility Principle).

Also, the operations are also easier to compose. The InitiateOrder operation may take one or more initial products and can simply invoke the AddProductToOrderOperation class to add them to the order.

[ServiceContract]
public interface IOrderService
{
    [OperationContract]
    InitiateOrderResponse InitiateOrder(
        InitiateOrderRequest request);

    [OperationContract]
    CommitOrderResponse CommitOrder(
        CommitOrderRequest request);

    [OperationContract]
    AddProductToOrderResponse AddProductToOrder(
        AddProductToOrderRequest request);

    [OperationContract]
    RemoveProductFromOrderResponse RemoveProductFromOrder(
        RemoveProductFromOrderRequest request);
}

[ServiceBehavior]
public class OrderService : IOrderService
{
    public InitiateOrderResponse InitiateOrder(
        InitiateOrderRequest request)
    {
        var operation = new InitiateOrderOperation();
        return operation.Execute(request);
    }

    public CommitOrderResponse CommitOrder(
        CommitOrderRequest request)
    {
        var operation = new CommitOrderOperation();
        return operation.Execute(request);
    }

    public AddProductToOrderResponse AddProductToOrder(
        AddProductToOrderRequest request)
    {
        var operation = new AddProductToOrderOperation();
        return operation.Execute(request);
    }

    public RemoveProductFromOrderResponse RemoveProductFromOrder(
        RemoveProductFromOrderRequest request)
    {
        var operation = new RemoveProductFromOrderOperation();
        return operation.Execute(request);
    }
}

public class RemoveProductFromOrderOperation
{
    public RemoveProductFromOrderResponse Execute(
        RemoveProductFromOrderRequest request)
    {
        // Implementation here.
    }
}

public class AddProductToOrderOperation
{
    public AddProductToOrderResponse Execute(
        AddProductToOrderRequest request)
    {
        // Implementation here.
    }
}

public class CommitOrderOperation
{
    public CommitOrderResponse Execute(
        CommitOrderRequest request)
    {
        // Implementation here.
    }
}

public class InitiateOrderOperation
{
    public InitiateOrderResponse Execute(
        InitiateOrderRequest request)
    {
        // Implementation here.
    }
}

Everything's a Service

Most web services I've seen implement a standard Data Transfer Object (DTO) pattern for their operations utilizing request and response DTOs or messages. The request message contains all of the information necessary for the operation to do its job, and the response message contains all of the operation's output. The behavior of a message class is limited to retrieving and saving its own data.

DTOs make unit testing much easier by increasing the visibility into what the operation does. Everything it needs to execute comes in the request, and the data the test needs to verify is returned in the response.

This works well for operations, so I thought why not use this pattern for business objects as well? Essentially each class can be treated like a service with its own well-defined contract and messages. Taking this idea even further, each public method could become stateless as private data members are moved to the request/response classes.

About two years ago I started converting business objects over to this pattern in one of my company's large web services. I even updated private methods to remove dependencies on class fields. Instead each method received these values as arguments. This year I continued this process, and it has worked very well. The guts of the service are now more unit testable and composable.

Separate Contract Classes from Business Logic Classes

I like to create a strict separation between service contract classes and implementation classes, and I'm accustomed to putting contract classes into their own assembly. Occasionally, I'll notice an implementation specific detail in the contract.  Sometimes it's a simple field to hold some temporary data.  Other times it's one or more methods with internal state. This drives me crazy because it becomes difficult to determine which data consuming clients are dependent on.

Below is an example of what I mean. It is based on something I came across recently. ShoppingCartItem is a contract class for a WCF service. It exposes Name and Price as data members. Two additional properties IsDiscountIncludedInPrice and IsPenaltyIncludedInPrice are ignored by the data contract serializer as instructed by the XmlIgnore attribute.  The presence of this attribute in a contract class is a clear code smell. To make matters worse, a couple of methods have been added.

[DataContract]
public class ShoppingCartItem
{
    [DataMember]
    public string Name { get; set; }

    [DataMember]
    public string Price { get; set; }

    [XmlIgnore]
    public bool IsDiscountIncludedInPrice { get; set; }

    [XmlIgnore]
    public bool IsPenaltyIncludedInPrice { get; set; }

    public void AddDiscount(Disount discount)
    {
        // Logic to update the price with a discount.
    }

    public void AddPenalty(Penalty penalty)
    {
        // Logic to update the price with a monetary penalty.
    }
}

To fix this, I did three things. First, I created a class called ShoppingCartItemImp (a terrible name I know) with all of ShoppingCartItem's members. Next, I removed the members in ShoppingCartItem that are not a part of the contract. Finally, I added a method to ShoppingCartItemImp that returns an instance of ShoppingCartItem.

The result is a new class whose implementation can change as the service logic evolves and a contract class free of implementation details.

[DataContract]
public class ShoppingCartItem
{
    [DataMember]
    public string Name { get; set; }

    [DataMember]
    public string Price { get; set; }
}

public class ShoppingCartItemImp
{
    public string Name { get; set; }

    public string Price { get; set; }

    public bool IsDiscountIncludedInPrice { get; set; }

    public bool IsPenaltyIncludedInPrice { get; set; }

    public ShoppingCartItem ToShoppingCartItem()
    {
        return new ShoppingCartItem()
            {
                Name = this.Name,
                Price = this.Price
            };
    }

    public void AddDiscount(
        ShoppingCartItemImp shoppingCartItemImp,
        Disount discount)
    {
        // Logic to update the price with a discount.
    }

    public void AddPenalty(
        ShoppingCartItemImp shoppingCartItemImp,
        Penalty penalty)
    {
        // Logic to update the price with a monetary penalty.
    }
}

Onward to 2013...

These are not the only lessons I've learned but the ones that have made the biggest impression. Others include decoupling logic with an event bus, finally getting comfortable with containers, and favoring automated tests over a UI for testing.

Here's hoping to an equally productive 2013.