Fluent Interfaces: Don't Chain for the Sake of Chaining

One of the goals of FEST-Assert 2.0 is to learn from the mistakes we made in the 1.x releases, even if that means not being backwards-compatible.

Not fully understanding the semantics of the API we were building, is, IMHO, one of the biggest mistakes we made in FEST 1.x. We were not able to see that each assertion in the method chain should be an independent unit.

To better explain my point of view, consider this snippet using FEST-Reflect‘s API:

Person person = constructor().withParameterTypes(String.class)
                             .in(Person.class)
                             .newInstance("Yoda");

In this example, each chained method in the fluent interface serve a common purpose: instantiate a new Person (similar to the builder pattern.)

On the other hand, in FEST-Assert, each method in the fluent interface has its own, individual purpose. For example:

assertThat(yoda).isInstanceOf(Jedi.class)
                .isEqualTo(foundJedi)
                .isNotEqualTo(foundSith);

The purpose of isInstanceOf is different than the one from isNotEqualTo. We can even call them individually:

assertThat(yoda).isInstanceOf(Jedi.class);
assertThat(yoda).isEqualTo(foundJedi);
assertThat(yoda).isNotEqualTo(foundSith);

In FEST 1.x I broke this assumption by introducing overridingErrorMessage as a way to override FEST’s default error message in case an assertion fails. Let’s take a look at this example:

assertThat(yoda).overridingErrorMessage("Yoda is a Jedi, dammit!")
                .isInstanceOf(Jedi.class)
                .isEqualTo(foundJedi)
                .isNotEqualTo(foundSith);

This is when it gets confusing. Now a method in the chain affects the behavior of the next one. It is hard to tell if overridingErrorMessage only applies to isInstanceOf, or to all the methods in the chain. It is so confusing that I cannot remember what were the semantics of overridingErrorMessage!

This is a potential fix:

assertThat(yoda).isInstanceOf(Jedi.class, overridingErrorMessage("Yoda is a Jedi, dammit!"))
                .isEqualTo(foundJedi)
                .isNotEqualTo(foundSith);

Now it is easier to understand that overridingErrorMessage only affects isInstanceOf.

Looking back, I can see that I introduced overridingErrorMessage the way I did because I naively thought that method chaining makes it easier to write and read code. It surely makes it easier to write code (just press “.” and your IDE’s content assist will show you all the available methods) but I showed you that chaining methods does not always produce a readable API.

In short: I abused method chaining.

Conclusion

When creating a fluent interface using method chaining, step back and think what are you trying to achieve. Do the methods in the chain share a common purpose? Are you chaining a bunch or independent methods? Regardless of the style you choose, be consistent and try not to mix them. That will make the code written with your API readable.

Oh BTW, we made the same mistake (again) in FEST-Assert 2.x. Luckily, there is still time to fix it :)