In one of my
previous posts about
Command-Query Separation (CQS) and side effecting functions being code smells, it was
pointed out to me again about immutable builders. For the most part, this has been one area of CQS that I've been willing to let break. I've been following
Martin Fowler's advice on method chaining and it has worked quite well. But, revisiting an item like this never hurts. Immutability is something you'll see me harping on time and time again now and in the future. The standard rules I usually do is immutable and side effect free when you can, mutable state where you must. I like the opt-in mutability of functional languages such as
F# which I'll cover at some point in the near future instead of the opt-out mutability of imperative/OO languages such as C#.
Typical Builders
The idea of the standard builder is pretty prevalent in most applications we see today with fluent interfaces. Take for example most Inversion of Control (IoC) containers when registering types and so on:
UnityContainer container = new UnityContainer();
container
.RegisterType<ILogger, DebugLogger>("logger.Debug")
.RegisterType<ICustomerRepository, CustomerRepository>();
Let's take a naive medical claims processing system and building up and aggregate root of a claim. This claim contains such things as the claim information, the lines, the provider, recipient and so on. This is a brief sample and not meant to be the real thing, but just a quick example. After all, I'm missing things such as eligibility and so on.
public class Claim
{
public string ClaimId { get; set; }
public DateTime ClaimDate { get; set; }
public List<ClaimLine> ClaimLines { get; set; }
public Recipient ClaimRecipient { get; set; }
public Provider ClaimProvider { get; set; }
}
public class ClaimLine
{
public int ClaimLineId { get; set; }
public string ClaimCode { get; set; }
public double Quantity { get; set; }
}
public class Recipient
{
public string RecipientId { get; set; }
public string FirstName { get; set; }
public string LastName { get; set; }
}
public class Provider
{
public string ProviderId { get; set; }
public string FirstName { get; set; }
public string LastName { get; set; }
}
Now our standard builders use method chaining as shown below. As you note, we'll return the instance each and every time.
public class ClaimBuilder
{
private string claimId;
private DateTime claimDate;
private readonly List<ClaimLine> claimLines = new List<ClaimLine>();
private Provider claimProvider;
private Recipient claimRecipient;
public ClaimBuilder() {}
public ClaimBuilder WithClaimId(string claimId)
{
this.claimId = claimId;
return this;
}
public ClaimBuilder WithClaimDate(DateTime claimDate)
{
this.claimDate = claimDate;
return new ClaimBuilder(this);
}
public ClaimBuilder WithClaimLine(ClaimLine claimLine)
{
claimLines.Add(claimLine);
return this;
}
public ClaimBuilder WithProvider(Provider claimProvider)
{
this.claimProvider = claimProvider;
return this;
}
public ClaimBuilder WithRecipient(Recipient claimRecipient)
{
this.claimRecipient = claimRecipient;
return this;
}
public Claim Build()
{
return new Claim
{
ClaimId = claimId,
ClaimDate = claimDate,
ClaimLines = claimLines,
ClaimProvider = claimProvider,
ClaimRecipient = claimRecipient
};
}
public static implicit operator Claim(ClaimBuilder builder)
{
return new Claim
{
ClaimId = builder.claimId,
ClaimDate = builder.claimDate,
ClaimLines = builder.claimLines,
ClaimProvider = builder.claimProvider,
ClaimRecipient = builder.claimRecipient
};
}
}
What we have above is a violation of the CQS because we're mutating the current instance as well as returning a value. Remember, that CQS states:
- Commands - Methods that perform an action or change the state of the system should not return a value.
- Queries - Return a result and do not change the state of the system (aka side effect free)
But, we're violating that because we're returning a value as well as mutating the state. For the most part, that hasn't been a problem. But what about sharing said builders? The last thing we'd want to do is have our shared builders mutated by others when we're trying to build up our aggregate roots.
Immutable Builders or ObjectMother or Cloning?
When we're looking to reuse our builders, the last thing we'd want to do is allow mutation of the state. So, if I'm working on the same provider and somehow change his eligibility, then that would be reflected against all using the same built up instance. That would be bad. We have a couple options here really. One would be to follow an ObjectMother approach to build up shared ones and request a new one each time, or the other would be to enforce that we're not returning this each and every time we add something to our builder. Or perhaps we can take one at a given state and just clone it. Let's look at each.
public static class RecipientObjectMother
{
public static RecipientBuilder RecipientWithLimitedEligibility()
{
RecipientBuilder builder = new ProviderBuilder()
.WithRecipientId("xx-xxxx-xxx")
.WithFirstName("Robert")
.WithLastName("Smith")
// More built in stuff here for setting up eligibility
return builder;
}
}
This allows me to share my state through pre-built builders and then when I've finalized them, I'll just call the Build method or assign them to the appropriate type. Or, I could just make them immutable instead and not have to worry about such things. Let's modify the above example to take a look at that.
public class ClaimBuilder
{
private string claimId;
private DateTime claimDate;
private readonly List<ClaimLine> claimLines = new List<ClaimLine>();
private Provider claimProvider;
private Recipient claimRecipient;
public ClaimBuilder() {}
public ClaimBuilder(ClaimBuilder builder)
{
claimId = builder.claimId;
claimDate = builder.claimDate;
claimLines.AddRange(builder.claimLines);
claimProvider = builder.claimProvider;
claimRecipient = builder.claimRecipient;
}
public ClaimBuilder WithClaimId(string claimId)
{
ClaimBuilder builder = new ClaimBuilder(this) {claimId = claimId};
return builder;
}
public ClaimBuilder WithClaimDate(DateTime claimDate)
{
ClaimBuilder builder = new ClaimBuilder(this) { claimDate = claimDate };
return builder;
}
public ClaimBuilder WithClaimLine(ClaimLine claimLine)
{
ClaimBuilder builder = new ClaimBuilder(this);
builder.claimLines.Add(claimLine);
return builder;
}
public ClaimBuilder WithProvider(Provider claimProvider)
{
ClaimBuilder builder = new ClaimBuilder(this) { claimProvider = claimProvider };
return builder;
}
public ClaimBuilder WithRecipient(Recipient claimRecipient)
{
ClaimBuilder builder = new ClaimBuilder(this) { claimRecipient = claimRecipient };
return builder;
}
// More code here for building
}
So, what we've had to do is provide a copy-constructor to initialize the object in the right state. And here I thought I could leave those behind since my C++ days. After each assignment, I then create a new ClaimBuilder and pass in the current instance to initialize the new one, thus copying over the old state. This then makes my class suitable for sharing. Side effect free programming is the way to do it if you can. Of course, realizing that it creates a few objects on the stack as you're initializing your aggregate root, but for testing purposes, I haven't really much cared.
Of course I could throw
Spec# into the picture once again as enforcing immutability on said builders. To be able to mark methods as being Pure makes it apparent to both the caller and the callee what the intent of the method is. Another would be using
NDepend as
Patrick Smacchia talked about
here.
The other way is just to provide a clone method which would just copy the current object so that you can go ahead and feel free to modify a new copy. This is a pretty easy approach as well.
public ClaimBuilder(ClaimBuilder builder)
{
claimId = builder.claimId;
claimDate = builder.claimDate;
claimLines.AddRange(builder.claimLines);
claimProvider = builder.claimProvider;
claimRecipient = builder.claimRecipient;
}
public ClaimBuilder Clone()
{
return new ClaimBuilder(this);
}
Conclusion
Obeying the CQS is always an admirable thing to do especially when managing side effects. Not all of the time is it required such as with builders, but if you plan on sharing these builders, it might be a good idea to really think hard about the side effects you are creating. As we move more towards multi-threaded, multi-machine processing, we need to be aware of our side effecting a bit more. But, at the end of the day, I'm not entirely convinced that this violates the true intent of CQS since we're not really querying, so I'm not sure how much this is buying me. What are your thoughts?