Search This Blog

Sunday, August 3, 2008

Builder's shouldn't mix

Those few who have read my past posts, may have noticed my preference for the use of builder style methods, which look something like this:

publisher.uses(anOutputStream).toPublish(aPublishableItem);



I like the expressiveness the builder style method gives me; Bringing me closer to chaining methods that resembled sentences.

While I like the way the above line reads, it's not enough to endorse their use any longer ... and here's why.

Publisher is a concept and like all good concepts, its definition is defined through an interface (or abstract class, but most likely an interface). If I was to look at this Interface, I'd see something like the following:

public interface Publisher
{

void toPublish(PublishableItem aPublishableItem);
Publisher uses(OutputStream outputStream);
}

The first thing that stands out is the uses builder method that takes an output stream and uses it in some fashion.

"What's so bad about that?" I hear you ask.

Let's assume I was not the author of this interface or any of its implementations. I would have to read the implementation to understand how this type was used. Or, more precisely, what sequence behavior should be called in, when required.

So it's possible that sequence of calls is very important. For instance (pardon the pun)

publisher.uses(anOutputStream).toPublish(aPublishableItem);


Could publish the publishable item to the passed in output stream.



Alternatively, I could have tacked the methods together differently:

publisher.toPublish(aPublishableItem);


publisher.uses(anOutputStream);


Might publish the publishable item to a default output stream and nothing to the passed in output stream.



What I dislike is that information about sequence of calls is only discernible in the implementation of our Publisher's behavior. That's an abstraction lower than I should have to go when reading clear, written code. The interface gives no clue.

One could argue the case, "Oh but anything that returns an instance of itself would be called first ... blah, blah, blah"; But, the fact still remains - One can't be sure by simply looking at the method signatures - and that extra trip to an implementation is a trip I'd rather not make if I didn't have to.

So what was the publisher refactored to?

After looking at an implementation, it was found that the publisher is created with a default output stream and if one is not provided at the time of publishing, the publisher defaults to the output stream created at its time of construction.

A middle ground was struck that didn't read quite as well as the first attempt, but provided an interface which told a better version of the truth than its predecessor.

To publish to the default output stream: publisher.publish(aPublishableItem);

To publish to a given output stream: publisher.publish(aPublishableItem, outputStream);

In summing up, I chose to favor clarity of type behavior, for developers who may want to use the Publisher type in future, over a nice sentence, for readers of existing code. In this case the trade-off is subtle, as the refactored code is still quite readable. What's important is that I value both readers of my code and users of my types. In doing so I try to satisfy both sides in a reasonable way. In this example, I felt that users of my type were unfairly disadvantaged to satisfy a really nice sentence like chaining of behavior.