Blocks and Glue

When writing Ruby libraries it is tempting to use code blocks as a way for the user to hook into the library or specify custom behaviour. This leads to a lot of code duplication, since blocks cannot easily be factored into a class hierarchy or mixins. Code blocks are no substitute for a good object model. Blocks are good for creating control structures and "glue" between objects, but should be refactored into methods or classes when they become longer than one or two lines.

When writing and using the Ruby dynamic mock library I used code blocks as the way that the programmer defined expected method signatures and checked expectations. For example:

mock.expect :set_property do |name,value|
    assert_equals( "Content-Type", name )
    assert_equals( "text/html", value )
    return nil
end

However, using blocks in this way has a number of disadvantages:

• They cannot be turned into a useful representation in error messages
• There is a lot of duplicated assert statements among expectations
• Expectations cannot easily be reused
• Expectations aren't represented as named concept, they just exist as undifferentiated lines of code

Constraints solved all these problems: they name what they are used for (e.g. IsEqual vs. IsSame), can easily be reused and combined (and, or, not), can describe themselves (toString). With Java dynamic mocks, the expectation above would be written as follows, where eq is a factory method that creates an IsEqual constraint.

mock.method("setProperty")
    .with(eq("Content-Type"),eq("text/html")).willReturn(null)
    .expectOnce();

Factoring out the concept of a constraint into the Constraint interface and various implementations gave us additional benefits for free: we could use Constraints to set up different expectations based on argument values, for example.

I've come to the conclusion that blocks are good for creating control structures and "glue" between objects, but should be refactored into methods or classes when they become longer than one or two lines.

Arose buy any other name wood smell as suite.

Programming is sometimes like writing poetry (although with perhaps slightly less social stigma): the choice of words is of utmost importance. Using the right word in the right place can make your code immediately comprehensible, even elegant. Conversely, choosing the wrong word can make code unbelievably complex. You have to be especially careful of words with more than one meaning, even more so when those meanings are similar but subtly different.

When writing the dynamic mock library I used the word "call" to refer to an incoming method call that was to be checked and given a mock behaviour by a mock object. A dynamic mock object intercepted each method call, created a Call object to represent the call, and passed it to a Callable object that checked expectations and stubbed their behaviour. A Callable had a method called "call" that took one argument, a Call called "call". Err... got that? Or are you getting as confused as I did? Perhaps some code will help...

interface Callable {
    public Object call( Call call ) throws Throwable;
}

Here's how it is called:

Call call = new Call( ... );
Callable callable = findMatchingCallable(call);
return callable.call( call );

That naming scheme is incredibly confusing. The word "call" is being used as a noun and a verb, to refer to a class, a variable holding a reference to an instance of that class, and to a method. And that's ignoring that "called" also means "named"!

As part of the refactoring of the jMock codebase we renamed Call to Invocation and the call method to invoke:

interface Invokable {
    public Object invoke( Invocation invocation ) throws Throwable;
}

Now the framework uses distinct nouns and verbs, and code is much easier to read. More importantly, members of a pair can talk about the code while programming without getting each other completely confused.

Don't test the methods of your classes

The most perfect expression of human behavior is a string quartet. — Jeffrey Tate

I recently had to work on some code I wrote before I had really understood test-driven development. I had written the tests first and the code had 100% test coverage, but the tests were not very useful in helping me understand the code.

The problem was that I had written tests for each method, testing pre- and postconditions and class invariants. This is the wrong approach to writing programmer tests. Instead, each test case should specify a describable aspect of the functionality an object provides to its clients. That aspect will probably involve multiple methods of the class. As a maintenance programmer using the tests as documentation you want to know how the behaviour of those methods is interrelated, not how each method acts individually.

Before writing that code I had recently written an application in Eiffel and the Design by Contract approach was fresh in my mind. This is one of the reasons I don't really like Design by Contract: it doesn't help you understand your code because the DbC specifications are attached to each feature of a class, instead of specifying aspects of a class' functionalty that each involve the interplay of multiple features. Using DbC to specify the interrelation of methods requires the programmer add features to the class that are otherwise unnecessary, and has the result of actually making the class harder to understand.

You can't make something out of nothing

Some time ago, in my incarnation as an academic researcher, I wrote a Java framework for animating 2D graphics called Scene Beans. While adding features to the framework I reused a Null Object class in a way that exposed the class to the user of the framework. This caused unexpected problems during later development. This taught me that Null Objects should not be used to model domain concepts but should be treated as internal implementation details of a framework and hidden from client code.

The SceneBeans framework provides a scene graph data structure that defines a graphical scene as a directed graph of polymorphic scene-graph "nodes" that are processed by Visitors. I used the Null Object pattern to mark the edges of the Scene Graph, and combined the Null Object and Visitor patterns so that the Null Object nodes did not double dispatch to the visitor — as far as the visitors knew, the Null Object nodes did not even exist.

One of the scene graph classes was a "switch" node that would contain multiple subgraphs but only draw one at a time. I later needed to selectively show or hide parts of the scene. I realised that I could do this by putting a subgraph and a Null Object node into to a switch node; switching between the subgraph and the Null Object would have the effect of showing or hiding the subgraph. This only required changing the Scene Beans file format so that files could explicitly specify Null Objects in the graph. Cunning!

Or so I thought...

I noticed a problem when we wrote a visitor to write a scene graph into a file in our format. The visitor never wrote the Null Objects because they did not double dispatch to it. This was fine for Null Objects at the edge of the graph, but was the wrong behaviour for Null Objects that had been explicitly created to be included in "switch" nodes.

The problem was that my modifications had changed the way that the framework used Null Objects, from using them as an internal implementation detail to mark the edges of the scene graph to using them to represent the concept of "draw nothing" in a user-visible way.

Solutions in reverse order of elegance:

1. Explicitly check for Null Objects in the save-to-file visitor. (A quick-and-dirty fix that I ruled out for obvious reasons)
2. Make the Null Objects call back to a visitNull method on the visitor interface, and provide a do-nothing default implementation of visitNull in the abstract base class from which all visitors are derived.
3. Use different classes for Null Objects that mark the edge of the graph from those used to represent "draw nothing".

This experience taught me that when changing code that uses the Null Object pattern, one must beware of modifying the system from using Null Objects as mere implementation details to using Null Object classes to represent domain concepts. This goes against the intent of the Null Object pattern and will cause headaches at later date.

Always make your tests fail

I ported some old code that I wrote in my branch of the mockobjects.com dynamic mock packages to the new jMock codebase. Looking through the test suite I found some tests that didn't actually test anything! They just passed without bothering to check the behaviour of the object being tested. Hmmm... I must have written the test and the code to be tested at the same time, and so expected the test to pass. Which of course it appeared to do with flying colours. What a stupid thing to do. I should have written each test, watched it fail and only then written the code to make it pass.

Lesson: always make your tests fail before you make them pass.

Or, as is already written on the C2 Wiki, "never write a line of code without a failing test.