Tag Archives: lmax

Declaring unit test bankruptcy

I have recently started a major stream of work centered on a particular application in the LMAX stack. This application has had plenty of features added to it over the last few years, but nothing has really required an overhaul.

Our work, however, is somewhat more involved; even finishing the simplest of our requirements has been taking a week or so – that’s a long time, for us.

Hitting the buffers

Our method, to begin with, looked something like the following:

  • Write acceptance tests for feature (we tend to batch these up – it helps us explore the story)
  • Write integration tests for our application, supporting the feature (these usually resemble the ATs)
  • Spike implementation within the application
  • Use knowledge gained from spike to drive refactoring
  • Repeat the last two steps until the ATs and ITs pass

We’re very much in the Kent Beck school of development here:

First refactor the program to make it easy to add the feature, then add the feature

Our problem was that refactoring the program was hard! We discovered that while making the ITs and ATs pass was easy, getting the unit tests to compile and pass was much harder.

This was frustrating; not least because the unit tests were of the overspecified mock interaction sort. If we moved even the smallest piece of validation, anywhere from one to a hundred unit tests would fail.

Symptom, not cause

We blamed the tests – they were stupid tests, we said; why had anyone bothered to write them? So, we tried to rewrite a couple of unit tests in a more lean style – just creating what we needed to test our new feature.

This felt a lot better right up until we finished, when we looked from our new tests to the old tests, and from the old tests to the new tests; but already it was impossible to say which was which.

These tests were a symptom that the code underneath was jumbled. Someone had attempted to break up large, core domain objects into separate responsibilities by pulling behaviour up into ‘processor’ objects, which had made things smaller but also broke encapsulation. More on this another day.

This was novel – here was a case where the wrong refactoring had painted us into a corner. The problematic tests this ill judged refactoring wrought besmirched all attempts to escape to a better place.

Declaring unit test bankruptcy

We decided to remove these unit tests. They were creating a catch 22 situation: we couldn’t refactor the code without breaking the tests, and we couldn’t make the tests better without fixing the code.

We ended up working like this:

  • Write acceptance tests for feature
  • Write one integration test for the application (a deliberately smaller step)
  • Spike implementation within the application
  • Run unit tests with spike code to detect pain
  • Rewrite those unit tests as integration tests
  • Delete the painful unit tests
  • Revert the spike, and use knowledge gained from spike to drive refactoring
  • Make new integration tests pass with well factored code
  • Continue until all the ATs pass

This allows us to make swingeing refactorings safely; speeding our journey towards a place where we may one day be able to TDD all the way down.

Enabling factors

We were lucky to have:

  • A mature integration tests framework. This made writing the new tests to assert only on the I/O events from our particular application easy.
  • A single threaded application (so integration tests are almost as quick as unit tests, and they don’t suffer from races)
  • Extensive AT coverage over the system as a whole.

Beware though, for these are double edged swords. Perhaps it is because our framework makes ATs and ITs so easy to create that we neglected the factoring of the code within.

It seems we have been guilty of declaring stories done when the acceptance tests all pass. If only life were that simple!

Reminder

In TDD, at the unit level, the method is as follows:

  • (write new test) Red
  • (make test pass) Green
  • Refactor

Here, ‘refactor’ is usually removal of duplication, and separation of responsibilities into separate classes.

We need to execute the refactor step ‘all the way up’.

The refactor step for ITDD and ATDD

I wrote a sort of checklist of things that I think about; but they were:

  • too specific
  • impenetrable
  • probably wrong

Instead, I advise instead that all one needs to do at this point is to stop and think. More specific advice is left as an exercise to the reader. (Hint: Think of the principles you apply at the unit level – can you scale them up to the level of systems and applications?)

Summary

  • Listen to your tests! We could have avoided this whole affair if we had listened to the tests at the time of writing.
  • Make sure your definition of done includes the ‘refactor’ step.

iOS First Impressions

Events have conspired against me and I find myself writing an iOS app. Off the back of some relatively complete android work, I was interested to see how the platform compared.

First steps

After the gigantic pain of upgrading a dual boot Leopard/Ubuntu macbook to Mountain Lion, installing XCode was relatively trivial (albeit I had to enter my password roughly five or six hundred more times than I’d hoped). I dived straight in hoping I would soon emerge with some useful knowledge; you will have to be the judge. Onward!

Objective C isn’t actually that weird

Once you figure out that Objective-C is seriously old (possibly even as old as C++!), some of its idiosyncrasies start to seem a little less odd.

Boolean is YES | NO

One assumes because originally, this was a macro based implementation (C doesn’t have a boolean type; or, at least, it didn’t until C99, where a standard macro implementation appeared, I think). This stackoverflow answer adds some colour.

There are no constructors 🙁

…and this has lead to a great disturbance in the force, with two idiomatic initialization methods.

  • alloc then init

    [[NDHSomeType alloc] initWithParams:params]

    which is tidy, but bad luck if you forget the init.

  • class methods that wrap this up for you

    +(NDHSomeType *) initWithParams:(NDHParams *)params

    which is tidier still, but cannot actually prevent someone still doing

    [NDHSomeType alloc] 🙁

This is very annoying as it hampers our ability to create truly immutable objects.

There are no namespaces

As such, XCode forces you to pick a three letter prefix for all of your class names. For shame.

Implementation hiding is a feature

This is probably my favourite thing so far. Given a Foo.h that contains

@interface Foo
@property NSString * bar;
@end

One can, in the implementation file, Foo.m, add further functions and properties as necessary, which are hidden from calling clients, thus:

@interface Foo()
@property NSString * hiddenFromClientsButVisibleInFoo
@end

This reminds me of the well used C++ pImpl idiom (see this post for a simple example), which I’m a big fan of; as such I’m pleased this exists in objective C too!

id

This appears to be the objective C equivalent of void *. Objective C is not that type friendly – collections are not generic, and calling any method you like on an id is only a compiler warning, not an error.

That said, one can, in a function definition, suggest that the actual object that is eventually passed adhere to one or more “protocols”, thus:

@protocol NDHStringListener
-(void) onString:(NSString *) someString
@end

@interface NDHStringTransmitter
-(void) transmitString:(NSString *) someString
        toListener:(id<NDHStringListener>) listener;
@end

Now we just need a class that adheres to this protocol:

@interface NDHStringDisplayer : NSObject<NDHStringListener>
@end

Passing instances of NDHStringDisplayer to NDHStringTransmitter is now fine. Annoyingly, passing instances of objects that don’t declare protocol adherence is still only a warning; although I assume one can configure the build process to make it an error.

Blocks

This is how objective C deals with anonymous functions. The documentation covers how this works, but essentially, things aren’t too bad; blocks are not too dissimilar from C++11’s lambda.

int (^addOne)(int someInt) = ^{
    return 1 + someInt;
};

int five = addOne(4);

a block named addOne, and an invocation of it.

One slight annoyance – I think all blocks are passed to other functions as id; there’s no way to say that you expect a block that takes an int and returns another, say.

Available APIs

Asynchronous execution

This is probably the most important part of the platform from my perspective – the app I want to write does a lot of I/O, so making sure this happens away from the UI thread is important.

iOS 4 introduced grand central dispatch (gcd), which seems highly sensible. It rather provokes the question “What the hell did people do before?”, but there you go. The abstractions on top of this don’t seem as rich as android’s AsyncTask framework, but that may be because the presence of blocks makes writing your own abstraction much simpler.

dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), 
^{
    NDHResult * result = [fn call];
    dispatch_async(dispatch_get_main_queue(),
    ^{
        [result passTo:receiver];
    });
});

An example of ‘dispatching’ a block to be executed in the background. On completion, a further block is ‘dispatched’ back to the main (UI) thread.

UI framework

The storyboarding provided in XCode makes it, I would guess, much easier for a lay person to get going in creating a layout. Android does have tools for this, but they are considerably more rudimentary, and any serious app will require some fiddling with the underlying XML.

Interestingly, storyboards seem to be relatively new, with many of the older iOS code examples still using a disparate collection of ‘xib’ files to create single views that are then linked together programmatically. As a beginner coming to the platform, this is somewhat confusing; which way is the right way?

I want to write quite a rich app, and I am finding that the storyboard is a bit of a crutch; I’m much happier with android’s HTML like layout model where I don’t really need to learn a complicated GUI tool in order to create a GUI! I suspect I may end up switching to the old style of xibs and doing the work the storyboard is doing in a more obvious fashion.

HTTP

Well, the provided framework is…quite low level; it feels quite like Java‘s HttpURLConnection (HUC), in that all it really wants to do is fling a byte array response at you with a few extra pieces of response metadata. This is, in fact, absolutely fine, as the included json library is perfectly capable of consuming that.

Predominantly, it also wants to behave asynchronously; I’d be interested to know if it’s powered by NIO underneath or a real blocking thread. (I am being evil and doing things synchronously on a background thread anyway, mwahaha.)

One huge upside is the lack of exceptions – each interaction may call you back with an NSError * or the resulting NSData * depending on success (synchronous methods ask for an output parameter in the form of an NSError **). This is way better than the clusterfuck of IOException sprinkled all over the Java equivalent.

Side note: one could compare the iOS HTTP library to Apache HttpClient, but that would be somewhat unfair, given the complete mess that android has made of deploying it, and their recommendation to use the HUC API for new apps.

Documentation and process

In general the API documentation is good – one can click through to see the provided header files and a brief google gets you to the equivalent web pages.

Some areas where Android is ahead:

  • StackOverflow answers are much better

  • Available examples tend to be richer, and more up to date …I suspect this is mostly because the “right” way of doing things has moved quite a long way on the Apple side, so it’s less clear what the idiomatic way of doing things is to the community at large.

  • Getting the app on to a real device is trivial for Android, where on iPhone, it’s a nine step marathon that you have to pay them $99 to participate in; this is not at all developer friendly and reminds me precisely why I don’t use iOS devices more generally.

XCode

All I will say is this: within half an hour I was googling to see if IntelliJ supported iOS projects yet. I shall have to give AppCode a try.

The Classpath

Before we begin – I really feel like “The Class Path” ought to be the name of a TNG episode. Perhaps something to suggest to the TNG_S8 twitter account.

Anyway…

There comes a point in every java program’s lifetime where it must answer once of life’s fundamental questions: how am I going to talk to my config file (or, indeed, any file like resource that’s not another class)?

As good test driven developers, we’ll usually have a short digression about the definition of unit testing before agreeing that there should be at least one test that reads from a real file in the same way that the production code will, and then start to write that test.

I have noticed a prevalence of projects that choose to use the src/test/resources area of a project in order to get the configuration file onto the classpath. The production code then reads that file from the root of the classpath and everything is fine. The discussion of whether this is the right thing to do appears to have happened long ago, unbeknownst to me, and that is a little problematic, because I am not convinced it is a good idea. I would much prefer it if most resources of this sort could live on the file system.

Why?

We’ll get to that shortly (if you must know the answer, click here), first though:

A little history

Perhaps readers have come across the concept of “maven layout”. This is a convention for arranging the source code in a project so that maven can easily understand and build it. Maven layout has become standard even in projects not built by maven, and that is, for the most part, no bad thing; in particular I particularly like that it allows for multiple languages within the same src tree.

One unfortunate introduction that maven makes, however, is the inclusion of src/main|test/resources. This is an area in which we can drop resources and expect them to turn up on the classpath without really thinking about it. As such, there is temptation to avoid answering the question of “where should we store this file” when there’s an easy get out of jail answer right in front of us.

This is fine right until the point where we stop using maven, resources in that directory no longer get bundled on to the classpath at the desired time, and a cacophony of alarm bells when we try to run our program or its tests.

An easy way out

When in maven layout, do as maven does: try to follow the classpath and assembly rules that maven applies to that filesystem structure accordingly. That’s not the point I want to make, however (and why use !maven if you’re going to make your specific !maven exactly like maven?), so forget I said anything.

Why is this bad?

..and why is the filesystem a better solution?

#1 Bad diagnostics

This is probably the one that sways me most. If we mis-specify a file path in our program, figuring out where it was supposed to be is as simple as logging the result of invoking getAbsolutePath on the java.io.File object that’s not finding bits where it should be.

What about loading a resource from the classpath? Well, a standard call to getResource will return us a java.net.URL. That sounds, initially, as if it might be quite useful. How about when the resource isn’t found, though? Well, in that case, we get back null. That is considerably worse than the file case; we have no idea where the classloader looked, and therefore no idea how to fix the problem.

If we’re lucky, we can get an idea of what the classpath is by looking in a debugger (sun.misc.URLClassPath is relatively easy to figure out). If we’re in a more complicated world with custom classloaders, however, this gets far more difficult.

[
file:/usr/lib/jvm/java-6-openjdk-common/jre/lib/jce.jar,
file:/usr/lib/jvm/java-6-openjdk-common/jre/lib/rhino.jar,
file:/usr/lib/jvm/java-6-openjdk-common/jre/lib/charsets.jar,
file:/usr/lib/jvm/java-6-openjdk-common/jre/lib/resources.jar,
file:/usr/lib/jvm/java-6-openjdk-common/jre/lib/jsse.jar,
file:/usr/lib/jvm/java-6-openjdk-common/jre/lib/compilefontconfig.jar,
file:/usr/lib/jvm/java-6-openjdk-common/jre/lib/management-agent.jar,
file:/usr/lib/jvm/java-6-openjdk-amd64/jre/lib/rt.jar,
file:/usr/lib/jvm/java-6-openjdk-amd64/jre/lib/javazic.jar,
file:/usr/share/java/java-atk-wrapper.jar,
file:/usr/lib/jvm/java-6-openjdk-common/jre/lib/ext/localedata.jar,
file:/usr/lib/jvm/java-6-openjdk-common/jre/lib/ext/sunpkcs11.jar,
file:/usr/lib/jvm/java-6-openjdk-common/jre/lib/ext/pulse-java.jar,
file:/usr/lib/jvm/java-6-openjdk-common/jre/lib/ext/dnsns.jar,
file:/usr/lib/jvm/java-6-openjdk-common/jre/lib/ext/sunjce_provider.jar,
file:/home/james/blogs/the_class_path/out/production/the_class_path/,
file:/home/james/idea-IC-123.94/lib/idea_rt.jar
]

A sample collection of paths from a UrlClassLoader.

#2 Ambiguity

So, let’s now imagine that we’re successfully running our program, and we want to know which config file is in use. We work out the list of places on the classpath, and find that config.txt is in three of them. Which one is in use? Yes, usually it is the one which is found first (just as it would be for an actual object file), but the order of finding is an implementation detail of a specific classloader, not a guarantee. With this ambiguity comes the dubious ability to override configuration by racing to put config.txt in the first listed place on the classpath.

With a file – well, there is a possible ambiguity in that if we specify the path to a file in a relative pattern, our lookup may be affected by the current working directory of the java process we’re in. If this error does occur though, we can trivially create a diagnostic message that tells us the absolute expected path of the resource we are seeking, and amend appropriately.

So what is the classpath for, then?

The classpath is a path, or collection of paths, where compiled java object files are found, just as LD_LIBRARY_PATH is a collection of places for the linker to look for shared objects. Wikipedia’s definition provides some more detail – and also avoids even the slightest hint of loading anything other than classes from it, to my delight.

Exceptions to the rule

Some java deployment platforms take the file system option off the table. Is this an excuse to start bundling non class resources on to the classpath? No.

  • The best ones provide better APIs for storage/retrieval of non-class resources.
  • A file is really only one type of URL. We could try loading configuration from a different protocol, like http.

Conclusion, and tl;dr for the impatient

The class path is for classes. While it does provide enough scaffolding to create general purpose resource loading solution, using it as such is error prone and unclear to both users and maintainers.

Snappier Conclusions

  • The clue is in the name.
  • The classpath. A path for classes.
  • This ‘filesystem’ concept. Might it be useful for storing files?

Close Quietly (with no exception)

A post about closing resources in Java. First though, an aside.

“Is it a good idea for destructors to throw?”

— Common C++ interview question

A quick bit of code can show us.

#include <iostream>
#include <stdexcept>

namespace {
  class Foo {
  public:
    void someUsage() {
      std::cout << "Using foo" << std::endl;
    }
    ~Foo() {
      std::cout << "Destroying foo" << std::endl;
      throw std::runtime_error("Exception 2");
    }
  };
}

int main() {
  std::cout << "Program started" << std::endl;
  try {
    Foo f;
    f.someUsage();
    throw std::runtime_error("Exception 1");
  } catch (std::exception const & e) {
    std::cout << "Caught exception!" << std::endl;
  }
  return 0;
}

Consider the above case where an object with a throwing destructor is popped from the stack. We naïvely hope our catch block will save us, but no, an exception has already thrown from another frame. We now have two exceptions to deal with and our program calls std::terminate (thanks to Alan for a correction here – I originally claimed this was UB).

Program started
Using foo
Destroying foo
terminate called after throwing an instance of 'std::runtime_error'
  what():  Exception 2
Aborted (core dumped)

This StackOverflow thread provides an excellent discussion of this issue from a C++ perspective. Many of the answers and comments gel nicely with the remaining content of this post, so it is well worth a read (but only once you’re done here, obviously).

What of our safe, garbage collected languages, though? What approach do we take to closing resources not managed by the VM?

If at first you don’t succeed…

try, catch, finally. This is the typical resource usage pattern in java – we acquire a resource (outside of the block – if we fail to acquire it, no close is required), use it in the try block, perform any desired exception handling in the catch block, and then clean up (usually invoking the close method on the object in question) in the finally block, which is (roughly) guaranteed to be called, regardless of what happens during try and catch.

Frequently the close method is marked as throwing a checked exception. This is annoying, as it means that even though we’ve performed some resource cleanup, our finally clause must also handle that exception type (or our function must propagate it upwards). Typically, we get around this by writing a closeQuietly method (or using one from an appropriate apache library) that catches the exception, suppresses it, then perhaps logs that a resource has failed to close.

This is absolutely fine for single resource usages – like reading all the data out of a file, or performing an http request to a remote server.

A more complicated world

Why’d you have to go and make things so complicated?

— Avril Lavigne

Commonly we will want to write applications that keep several resources of different species open for the lifetime of our application (or at least, considerably longer than a single function call). Perhaps a collection of sockets, some shared memory accessed through JNI and a sprinkling of semaphores.

What happens when we want to cleanly stop our application? These different resources will presumably be looked after by different pieces of our object graph, so we will have to traverse that graph calling close where appropriate.

In a perverse world, our code might look like this:

    public static void main(String[] args) {
        final SharedMemory memory = new SharedMemory();
        final CustomSocket customSocket = new CustomSocket();
        try {
            runApplication(memory, customSocket);
        } catch (Exception e) {
            e.printStackTrace(System.err);
        } finally {
            try {
                memory.close();
            } catch (SharedMemoryException e) {
                e.printStackTrace(System.err);
            }
            try {
                customSocket.close();
            } catch (SocketCloseException e) {
                e.printStackTrace(System.err);
            }
        }
    }

This may be a rather extreme example. Neither resource implements java.io.Closeable (which would make things easier as we could extract a single method), and both give feedback only in the form of a checked exception. I’ve left the try/catch blocks in place to illustrate just how annoying this is. How much worse this gets per different resource species is left as an exercise to the reader – one hopes it is obvious that this is another place that exception throwing fails to scale.

An alternative approach

We could mandate extension of Closeable for all such resources, but all this buys us is the ability to have just one static closeQuietly function. That would be a start. Can we do better though?

Groans from the crowd. Is this another anti-exception post? You bet it is. Let’s consider this alternative interface:

package net.digihippo;

import java.util.Collection;

public interface Closeable {
    public Collection<? extends Exception> close();
}

A couple of things to note here before we continue.

  • The interface isn’t actually that important – here’s the previous program with resources that follow this pattern without implementing the interface:
    public static void main(String[] args) {
        final SharedMemory memory = new SharedMemory();
        final CustomSocket customSocket = new CustomSocket();
        try {
            runApplication(memory, customSocket);
        } catch (Exception e) {
            e.printStackTrace(System.err);
        } finally {
            final List<Exception> closeProblems = new ArrayList<Exception>();
            closeProblems.addAll(memory.close());
            closeProblems.addAll(customSocket.close());
            for (final Exception e: closeProblems) {
                e.printStackTrace(System.err);
            }
        }
    }
  • Exception may not be the type that we want here. Once we have a return type, it’s possible that we just want a Collection of String messages that tell us why close has failed, rather than the considerably more expensive Exception. I chose it mostly because it was nearest, and possibly because having the stack could be useful for post-hoc rationalization.

These particular resources, in our example, share the program’s lifetime, and so the only customer for the close mechanism is the programmer trying to correctly order a whole bundle of unrelated shutdown and close calls. We will make their life a complete misery if we take the lazy option of failing to close by throwing – what on earth do we expect our shutdown writer to do with it?

Conclusion

For resource library creators

C++ has given us a good hint – throwing from close functions (and other functions in that family, like stop and shutdown) is unhelpful. In particular, throwing a custom checked exception type from a close method verges on malicious. It makes writing shutdown code tedious, and tedium is not a fate we should wish on anyone. We should do our brave shutdown hook writers a favour and provide our error output in a sanitized format. If we really, really must throw (and by goodness we need a superb excuse to do so), implementing java.io.Closeable is mandatory.

For shutdown writers

We are in a pretty poor place right now. From a brief scan of libraries I use, those pesky resource library creators (Presumably this includes the author? – Ed) have been out to get us – almost all of their close methods throw checked exception types that we have no hope of recovering from. Try to wrap their laziness as close to the source as possible; who knows, perhaps the idea will take off, and our lives infinitesimally improve.

Postscript

Having written no C++ for some time, I fashioned the top example without having to look up what header std::runtime_error was in, and the program compiled and ran at the first attempt. Get in. Admittedly, it didn’t do what I thought it would (my Foo was optimized away), but even that small victory made me smile. Having written this rather smug post-script I now eagerly await -Wpedantic like feedback from less rusty C++ writers!

Object Creation Patterns #2 : Everybody’s building

The right time to build

We’ve talked at length about the costs of builders, and yet… they persist. I can think of at least three examples from popular java libraries that work well:

Although, having looked at StringBuilder‘s doc – augh! It’s so big! Five or six builders could come out of that, starting with AppendOnlyStringBuilder.

What do these master builders (That’s enough linguistic hyphens between the Builder pattern and actual builders – Ed) have in common?

Well, they all have trivial build implementations. Really though, that’s an artifact of the thing that they’re building – there’s a well defined null type for each – Description‘s is simply an empty description, StringBuilder‘s is "", and ImmutableList.Builder‘s is simply ImmutableList.of(). All three of these things are essentially list builders: Description and StringBuilder are looking after [Char], and ImmutableList.Builder is looking after some appropriate [T].

This gels well with good experiences I have had with builders in code I’ve been near: applications with publishers and subscribers, tuples with arbitrary numbers of arbitrarily typed fields, user lists built from clicks in a UI, before a batch action is selected. When you hit build on all of these things, the type you get back is either [T], or a type that depends heavily on a [T]

Careful now.

We could quite easily conclude here that the only good builders are those that build lists. The original point bears repeating:

Builders work best when there’s a well defined null type for the thing being built.

Even that, though, is not quite right. It’s quite possible for a Builder to work nicely if the thing being built is a composite of several types, each with a well defined null type. The following definition might be better:

Builders work best when it’s impossible for them to be in a state where invoking build generates an invalid object

That feels like a corollary of this, though:

Builders work best when all of their fields have a well defined null type

In addition, the interplay between the dependencies should also have a well defined null type. Defining that, however, is tricky, so I will assume we’re all one the same page and gracefully move on to defining it for single fields…

A well defined null type

Some examples:

  • List‘s null type is the empty list
  • String‘s null type, is, unsurprisingly, an empty String, given that a String is just a List of chars.

Arbitrary objects do not have a well defined null type. null in Java is not well defined, because at some point it’s either going to cause NPE, or a giant tangle of obj == null ? doStuff(obj) : panic(); cruft. Using null to signify emptiness or default values in builders is a crime I hope no-one is tempted to commit.

So, each field can either be a primitive, for which a default value can be specified, an interface, for which a default implementation is specified (perhaps a no-op one), or a type like List, Iterable or Maybe (these have well defined null types). We could start talking about builders within builders here, but we won’t.

Cowboy Builders (You’re fired – Ed)

It’s difficult not just to define these as being !good builders. So instead, here are some things we may see if a builder is being used inappropriately (I say!):

Unnecessary builders

Fewer than four fields. One usage. Inline with extreme prejudice.

Irresponsible builders

Built objects propagate across the system after construction, and each part of the system does some more validation of the object to see if it is suitable.

Perhaps each receiver needs a slightly different builder? Perhaps they all need the same builder, and we’ve found a case where a non trivial build implementation is required, which performs the validation just once.

Alternatively, this could be because of null or sentinel defaults (Long.MAX_VALUE, anyone?) leaking out from the built object. We can either establish a sort of null protocol at call sites or find a more appropriate default.

A simple example null protocol for Maybe unfriendly lingoes: null can never escape. hasField is obvious and is elided.

void doThingWithFoo() {
   if (hasField()) doThing(getField()) else doThing(default());
}

Foo getField() {
   Preconditions.checkState(hasField(), "go away!");
   return fooField;
}

Overexposed builders

Often, the fluent style of building is used to mimic the sort of pass-by-name arguments we see in more fun programming languages:

my_burrito = burrito(size: "large", filling: "pork",
                     beans: "both", salsa: "hot",
                     guacamole: "yes please")
final Burrito myBurrito = newBurritoBuilder().withSize(Size.Large)
                                             .withFilling(Filling.Pork)
                                             .withBeans(Beans.Both)
                                             .withSalsa(Salsa.Hot)
                                             .withGuacamole()
                                             .build();

So. This is the first quandary I find myself in. I really like this style, it’s descriptive, leaving me confident that the constructed burrito will be as requested. How many of these fields really have a well defined null type, though? What are the mandatory requirements for a burrito? Could we just use a builder for the optional parts? Perhaps the following might be more appropriate:

Burrito myBurrito =
    new Burrito(Size.Large, Filling.Pork, Beans.Both,
                options().withSalsa(Salsa.Hot).withGuacamole().build());

We should carefully match the fields we use a builder for based on whether the object about to be constructed really needs them or not. Exposing the mandatory dependencies as builder parameters is potentially confusing and costly.

As my friendly, local ruby guru points out though, you can get the nice fluency of this pattern without the cost:

Broadly I agree with this. I have seen times where it works beautifully, particularly in creating objects in test. There have also been times, however, where I have come across that pattern, felt it inappropriate and simply inlined until the builder disappeared.

Conclusion

We now have a reasonable guideline for when to use builders, and some example smells which might allow us to spot a cowboy more quickly (I give up – Ed).

Where now?

If this series manages the difficult third album, we’ll perhaps start to touch on more general object creation concerns, and perhaps a little bit of wiring.

Object Creation Patterns #1 : Factory beats Builder

Well, really, this should be obvious. My mental image of factories is the Toyota plant; an efficient production line of high tech robots automatically executing specialized constructive tasks as part of a well honed process (perhaps with a nearby overseer sternly observing, making regular marks on some sort of handheld checklist device). Whereas my builder image is a bloke leering at young ladies from the safety of some dodgy looking scaffolding.

Ahem

Uh. Yeah. I actually wanted to talk about the Factory and Builder patterns. They’re inescapable in modern OO codebases. I want to talk about the right time to use both patterns. In this part, we will discuss why Factory ought to be the default, and Builder‘s niche is, well, a niche. Later posts will examine the niche further, and probably digress into wider object creational concerns.

A little discipline

Alright, alright. We should define these terms.

Factory

Given a class Foo, a given Factory is a one shot method to construct a Foo.

Rule of thumb

Once we have a Factory, we are precisely one method call away from having a Foo. Factory objects might contain some pre-bound dependencies of Foo that they can inject on invocation, but usually this has occurred long before the invoker came into being.

Side note – Factory methods, usually implemented as static functions on Foo definitely get pulled in by this definition. (Q: what about Foo‘s constructor? Is that a Factory too?)

Builder

Given the same class Foo, a Builder allows us to piece together the dependencies of Foo in its internal state until we are ready to construct our Foo by invoking the build method.

Rule of thumb

Once we have a Builder, we could be n + 1 calls away from having a Foo, where n is the number of constructor parameters Foo has.

A completely irrelevant equivalence

One can immediately see that one could generate a Factory for a given Builder usage. Somewhat more generally, any Factory could be replaced with some equivalent calls to a complete Builder for a given class.

Takeaway from this section: Builders are more ‘general’ than Factories.

Wait a minute. Are you going to argue for or against generality?

Yes. What we are really seeking here is the appropriate level of generality for the situation we are in.

Factories are low risk; one might say they have low entropy. All the arguments we need in order to create a new object are clearly listed, passing the wrong number and/or the wrong type of arguments and the compiler will (assuming null is verboten) let us know long before the missiles have launched.

Builders come at a far higher cost; just as their construction is at least two phase, their contract is split too, between compile and run time. N.B The cost goes up if the builder’s implementation uses mutable fields, as they commonly do!

We can know fairly well that each of our calls parametrizes some aspect of the object under construction correctly, but do we know if all the dependencies it needs to construct our object have been provided when we invoke build? The only time we find out is at runtime, which may be too late. Let’s examine Builder from three different perspectives.

Using a Builder

We should be sure to write a test for our particular construction. If our usage is based on values only known at runtime, that might be quite a few tests, and a bundle of validation logic for our inputs. Ouch.

Writing a Builder

We’ll have a lot of fun considering the possible default values for fields, writing the validate method, and putting together the documentation that gives our clients the faintest chance of not failing it. You know the sort of arduous fun I speak of here, I am sure.

Receiving ‘built’ objects

We will be fortunate if we can work out all the different species of object that might occur! We’re probably going to want to use a property based testing tool to attempt to cover the vast range of objects we might end up dealing with. If we’re really lucky, a kind person might have hidden the built value behind a well defined interface that we can test against instead. I don’t know about you, but I don’t tend to be that lucky.

Conclusion

One hopes the above example gives some weight to the message of this piece, i.e there must be a strong need for the sort of generality a builder provides given the costs that come along with it.

Coming up next time: some strong needs, and master builders versus cowboy builders

Haskell’s benevolent influence

without even a mention of the ‘M’ word

One of the things that attracted me to SPA 2013 was the Neural Net workshop on the Sunday. It turned out to be even better than I hoped; although we didn’t get quite as far as we could have done.

The first thing we wanted to do was implement a Neuron. In this particular case, a neuron took two inputs (let’s say they are doubles, for the time being) and then performs a computation on those inputs to determine a single output.

In this case, our neuron’s computation was as follows:

output = inputOne * weightOne + inputTwo * weightTwo > threshold ? 1.0 : 0.0

My pair and I quickly put together an implementation of this in Java that looked a bit like this:

package net.digihippo;

public class NeuronImpl implements Neuron {
    private final double threshold;
    private final double[] weights;

    public NeuronImpl(double threshold, double[] weights) {
        this.threshold = threshold;
        this.weights = weights;
    }

    @Override
    public double run(double[] inputs) {
        double sum = 0.0;
        for (int i = 0; i < weights.length; ++i) {
            sum += (weights[i] * inputs[i]);
        }
        return sum > threshold ? 1.0 : 0.0;
    }
}

You could obviously implement this as a static function, but suitable hints were given that the threshold and weights were properties of a given neuron, and so fields it was.

Many connected neurons – a neural network

To cut a long story short; we eventually want to plumb several neurons together to form a network. A single neuron is limited in the functions it can express, for reasons I won’t explore in this post; networks of them do not suffer in the same way.

Handily, once the weight and threshold of a given neuron are fields, the call to a neuron matches that to a network of neurons. But we needed a way to plumb a tree of neurons together, like so:

inputOne ->|          |
           | neuron 1 |_  
inputTwo ->|          | \_|          |
                          | neuron 3 |---> output
inputOne ->|          |  _|          |
           | neuron 2 |_/ 
inputTwo ->|          |

N.B Each neuron on the far left of the network is passed the same pair of inputs.

We can see that each Neuron either has precisely two ancestors or none at all (proof by terrible ASCII picture + some induction). A light, well, actually it was more a series of small, excited explosions, went off in my head. Now I just needed to remember that Node a Tree a Tree a magic and I would be laughing. My pair was working in Java, however, and somewhere between the magic and the .java files being edited in Eclipse, something wasn’t working. I needed to get it onto the screen before I could translate it across. Ten minutes or so later, I had this Haskell version of Neuron (which incorporates the concept of a network):

data Neuron = Neuron Double Double Double
            | NeuralNetwork Neuron Neuron Neuron
              deriving (Show)

runNet :: Double -> Double -> Neuron -> Double
runNet i1 i2 (Neuron t w1 w2) = bool2doub $ (i1 * w1) + (i2 * w2) > t
runNet i1 i2 (NeuralNetwork n n1 n2) = runNet n (runNet i1 i2 n1) (runNet i1 i2 n2)

bool2doub :: Bool -> Double
bool2doub True = 1.0
bool2doub False = 0.0

Side note: Feedback on any Haskell naïveté within this post would be greatly appreciated!

As you can see we have a neat, recursively defined, data type, and we defer any actual neuron run until we reach a “leaf” (which might have been a better name for the first constructor) at the far left of the ASCII diagram from before. It should be possible to translate this to Java without too much difficulty; we can map the data type Neuron to an interface, and the two constructors to two implementations of that interface.

Compiling Haskell to Java via human.

The sharper eyed readers will have noticed that we were on the way to doing this already in the Java source; NeuronImpl implements Neuron. An accidental bit of foreshadowing there, apologies. So, our NeuronImpl doesn’t have to change, but what does NeuralNetwork look like?

N.B Apologies for the use of above and below – they are extremely poor names for the two parent neurons.

package net.digihippo;

public class NeuralNetwork implements Neuron {
    private final Neuron neuron;
    private final Neuron above;
    private final Neuron below;

    public NeuralNetwork(Neuron neuron, Neuron above, Neuron below) {
        this.neuron = neuron;
        this.above = above;
        this.below = below;
    }

    @Override
    public double run(double[] inputs) {
        return neuron.run(new double[]{above.run(inputs), below.run(inputs)});
    }
}

Remarkably similar, it turns out! The Haskell version is somewhat terser, but the idea appears to have survived translation. It’s quite pleasing to know that lessons from Haskell can (albeit in this simple example) be applied in less fun languages. One wonders if there are places where we cannot translate Haskell data types and their constructors into interfaces and their implementations.

A new requirement arrives

Two inputs? Bah! We laugh in the face of two inputs. We want to be able to deal with n inputs. N.B This means that each level of the network has n times more neurons than the next. Perhaps this might be where our Haskell -> Java translation starts to creak? Let’s add the idea of multiple inputs to our Haskell implementation.

data Neuron = Neuron Double [Double]
            | NeuralNetwork Neuron [Neuron]
              deriving (Show)

runNet :: [Double] -> Neuron -> Double
runNet inputs (Neuron t weights) = bool2doub $ (sum (zipWith (*) inputs weights)) > t
runNet inputs (NeuralNetwork n ns) = runNet (map (runNet inputs) ns) n

Nice and simple – if anything, the variable input version is actually simpler than the fixed, two input version. The use of sum (zipWith ... is particularly pleasing, to my eye. Let’s run it through the mangler and see what we get out in Java…

package net.digihippo;

public class NeuralNetwork implements Neuron {
    private final Neuron neuron;
    private final Neuron[] parents;

    public NeuralNetwork(Neuron neuron, Neuron[] parents) {
        this.neuron = neuron;
        this.parents = parents;
    }

    @Override
    public double run(double[] inputs) {
        final double[] results = new double[inputs.length];
        for (int i = 0; i < inputs.length; ++i) results[i] = parents[i].run(inputs);
        return neuron.run(results);
    }
}

Not bad

Conclusion

Translating a well meaning Haskell implementation into Java:

  • can work
  • might even be a good idea.
  • involves writing some Haskell, and is therefore good for you.

A horrible warning

There is recursion here. Mind how you go.

A laziness trick to relax your code

So, this week on Distilled Derivatives, some more laziness. And also, exceptions. Everyone loves them, really useful things for letting you move error handling into a single location that they are. Now and again though, their misuse can really get in the way of getting something done.

Retrieve the content of the first functioning url

Given n urls in some ordered collection, get the content of the first “working” url, where working is defined to mean “has http code 200 and non empty response content”. Once you’ve found it, put the response body of the url into some local file.

This should be easy, thought I. How wrong I was.

First problem

All the java http libraries throw IOException. Ok, that’s not too bad, we can get past that with only a little evil. Here’s our first go.

  def withUrlConnection() : String = {
    for (url <- urls) {
      try
      {
        val actualUrl = new URL(url)
        val connection = actualUrl.openConnection().asInstanceOf[HttpURLConnection]
        return stringFromHttpUrlConnection(connection)
      }
      catch {
        case e: IOException => {}
      }
    }
    throw new RuntimeException("all failed")
  }

This isn’t too bad – we’re only teetering on the edge of breaking the “don’t use exception for control flow” rules, rather than falling into the abyss. We can simply take the resulting String and write it to a file (implementation elided). Oh, and in case you were curious, here’s the implementation of stringFromHttpURLConnection (we’ll need it again later).

N.B I’ve completely elided any closing of any connection everywhere in this article, because like the technique I’m trying to describe, I am lazy.

  val stringFromHttpUrlConnection: (HttpURLConnection) => String = { connection : HttpURLConnection =>
    Source.fromInputStream(connection.getInputStream).getLines().mkString("\n")
  }

The problem worsened, however, when some kind soul tried to help out by creating a function like the following:

Problem 2 – an unhelpful helper

  def withUnhelpfulFunction(path: String) {
    var done = false
    for (url <- urls) {
      try {
        if (!done) {
          downloadFile(url, path)
          done = true
        }
      } catch {
        case e: IOException => {}
      }
    }
  }

  def downloadFile(url: String, path: String) {
    // lobs url response body into path on success, IOException if not. Implementation too boring to include.
  }

At first glance this function looks like it might save us some code, but once you start talking about failure or multiple urls, we’re screwed. It’d be perfect if we wanted a one shot download that noisily failed, and that’s probably the use case the original author had in mind. In general the approach the function takes is a bit evil – it’s performing IO and letting us know we succeeded by not throwing an exception. That’s not the world’s greatest signal.

So, two quite ugly solutions with exceptions. Is there another way?

My hobby: getting lazy evaluation to write my program for me

Functional programming has given us some excellent tools to help decompose this problem, and there’s a neat lazy evaluation trick that can get us out of jail here. What do we need? A function that will:

  • synchronously resolve a url to the response body
  • include the possibility of failure in its return type

Roughly, this is equivalent to URL -> Maybe String (or Option[String], in Scala land). Try as I might, I can not find a scala or java http client library that follows this approach. Dispatch looks terrific but is also asynchronous, and I don’t really want to talk about futures in this post. Handily, numerous other sandbox projects I’ve tinkered with have already forced me into writing my own, so I have a handy function lying around.

  def withHttpLib() : String = {
    val requestor: HttpRequestor = new InsecureHttpRequestor()
    val results = urls.map(url => requestor.performGet(url, stringFromHttpUrlConnection))
    results.find(_.wasSuccessful()).getOrElse(throw new RuntimeException("all failed")).getResult
  }

Look ma, no for or if! Even better, this approach appears much shorter, however, this is mostly because much of the responsibility has moved into the little http library. Let’s walk through the execution of this program.

A little secret

  val urls = Stream("www.digihippo.net/home.html",
                    "www.digihippo.net/index.php",
                    "www.digihippo.net/foo.bar")

The structure the urls are sitting in is actually Scala’s excellent Stream abstraction. By default, operations on Streams are applied lazily. For example, if we invoked map on a particular instance of Stream, execution of the function we pass in is deferred until someone tries to extract an element from the Stream. In our earlier examples, we use Scala’s for comprehension to iterate across urls. In our favourite implementation, you can see that we’ve started applying map to it instead.

N.B These won’t actually work if you do new URL(url) on them. They’re in the current format because my library doesn’t yet work out whether to connect securely or not from the passed in url String.

What’s this http library doing then?

    val requestor: HttpRequestor = new InsecureHttpRequestor()

The requestor here is an object from my http library. It’s insecure because it doesn’t deal with https (it doesn’t lack confidence), just so you know. To execute a given request, one provides a url, and a response decoder – the requestor returns you a type parametrized Http object, which may contain the thing you wanted to decode, or an error. What’s happening underneath? Well, something like this:

  • Attempt to get url
  • On failure, return Http.failure() with some helpful details
  • On success, attempt to parse using provided response decoder
  • If response decoder succeeds, return whatever it decoded
  • If it failed, return Http.failure() with some helpful details

Hold on though – why does our decoder get the opportunity to return any sort of Http? Why doesn’t it just attempt a parse and give up? Well, we have to really think about what makes any given http request a failure. The definition in this post is not necessary universal – sometimes we expect a 404, or a 500, and still wish to parse something from the resulting connection.

As such, at the moment, the decoder can examine any aspect of the created HttpURLConnection and decide for itself, if it wants to, that this request was a failure. Our decoder does no such thing though – it just attempts to grab the response body and whack it into a string. The library will helpfully catch any exception resulting from playing with HttpURLConnection and return you an unsuccessful Http instance with the error information stashed inside.

What is this line really doing?

    val results = urls.map(url => requestor.performGet(url, stringFromHttpUrlConnection))

So, hands up – who thinks we’re firing off http requests as we invoke map here? No-one? Good. Glad to see people are keeping up. What we’re really doing here is creating a new Stream, which at the moment is actually a collection of functions that, when called, will return an Http[String]. So this line actually has no side effects, we’re really just defining how to compute the next elements of results, rather than computing them.

This is where the magic happens

    results.find(_.wasSuccessful()).getOrElse(throw new RuntimeException("all failed")).getResult

Now, the density of this line is considerably higher than the others. If that’s a problem for you, you might be a n00b. So, what the hell is happening?

Well, that find invocation would be better named findFirst. It traverses results, examining each element to see if it was a success. That means it needs to compute the element! So, find actually does this:

  • Execute function to obtain element. N.B this finally executes our map function
  • Examine result to see if it was successful
  • If not, repeat with next element

So, in fact, just by defining results in the way that we have means that find expresses the control flow of our application for us. One could legitimately criticize the first two solutions for repeating this standard library code, and, in fact, functional languages go out of their way to make extracting this sort of legwork easier. If you’re wondering why this is important, measure how many times you write try/catch/finally in java (or similar) next week.

The remainder of the line does the necessarily evil to fulfil something like the original contract, where we wanted a String that we could later write out to a file. We need to extract the actual value from the find result (find actually gives us back Option[Http[String]], because there’s no guarantee any of our results matched), throwing if none were successful, and then we need to extract the String out of the found successful request.

An apology

Purist functional programmers will likely be groaning at this point. The side effects of our little getter don’t happen at the top of our program, necessarily, and the usage of Option (and the Http type, which is pretty similar to Option) is not particularly idiomatic (hitting get like functions on monadic types is frowned upon).

Some side notes

Interestingly this very same function was implemented as part of a bash build script elsewhere, and it embodies a lot of the lessons that we’ve seen in the code here. Here’s a cut down equivalent.

#!/bin/bash

function noisy_curl() {
    echo "issuing request to get $1"
    return `curl -s -f $1`
}

noisy_curl http://www.digihippo.net/main.html ||
noisy_curl http://www.digihippo.net/index.php ||
noisy_curl http://www.digihippo.net/site.html

This is, given how much effort we’ve gone to set up the same effect in Scala, embarrassingly trivial. One can start to see the elegance of the unix way of doing one simple task, and writing the result to stdout, while also indicating success with a return code. Here we use the -f flag to tell curl to ‘fail’ if we get a suitably erroneous looking response code, and then just set up a series of curl invocations such that the first success short circuits the others. One starts to agree with all those people on the internet who think UNIX pipelining looks a helluva lot like composition as seen in functional programs.

We’ve come a long way baby.

So let’s stop (for now). We could certainly expand on our example by doing something more interesting that putting that result String into a file. We could consider approaches where we never parse the whole String but instead hold on to the InputStream instead. These deserve to be posts in their own right though, so we will not discuss them further here.

What did we learn?

  • Lazy evaluation allows you to elide boring control flow from your code. More generally, a functional approach can help you separate the valuable parts of your code from arduous syntactic toil
  • Wrapping exceptions into return types at the point where they are thrown makes life so much easier
  • Communicating errors in exceptions is really only good for one shot calling patterns

…and, again, more generally, exceptions are vastly overused. How often per day do you get a broken web page when browsing? Is it really exceptional? Are browsers dealing with 404 or 500 via an exception path? I seriously hope not. Exception abuse seems particularly prevalent in java, perhaps because so many libraries use IOException as their IO alarm, and possibly also because the language makes it so difficult to work with something like the Http type. Once again, I find myself thinking that Java 8 really cannot arrive soon enough.

Moral of the story

Stop throwing all that stuff around and relax, dude.

ThreadLocal<NotMyCupOfTea>

tl;dr ThreadLocal or ‘beautiful architecture’. Pick one.

Now. Don’t get me wrong. I’m rewriting this article for around the third time, and my opinion has softened on each attempt. To rebut my original tl;dr: ThreadLocal certainly isn’t anywhere near as bad as Singleton! They lack Singleton‘s most glaring evil – the global access. Someone needs to hand you the right ThreadLocal<T> in order for you to get the T out; you can’t just do ThreadLocal::get and magically have the thing you want.

That’s not to say that ThreadLocal is good, however. In most systems one would argue that an opportunity to express the threading model of your application in the object model has been lost. Let the code speak, and let it speak in the tongues of a dependency graph of objects that is well formed (or at least acyclic)!

To aid you in your quest of object/threading model sympathy, herefollows a critiqued example of potential ThreadLocal usage.

It’s that canonical java problem! SimpleDateFormat is not thread safe.

Imagine the scene. We’re in a java web server, and thousands of clients are trying to access pages and pages of content that urgently need some dates to be formatted. Unfortunately, some poor soul missed the relevant javadoc that explains the not thread safeness of SimpleDateFormat, and some of those web pages are coming out with dates in 1970.

The horror. We could very easily solve this with a ThreadLocal<SimpleDateFormat> and arrange for our servlets to have access to it. At the point where we need to format a date in servlet code we just grab the formatter out of the ThreadLocal and use it. Frankly, that’s not the worst thing we could do here. Date formatting is unlikely to be the raison d’etre of your program, so promoting it to a first class threading concern could be overkill.

A little deeper

Let’s imagine we really care about this though (not too tricky for me, your mileage may vary).

We could use JodaTime instead (or the new spingly new Date/Time stuff coming in java 8). For this particular problem this is probably the most sensible option. If we think about the contract of format, it really shouldn’t need a shared resource in order to function. It came as no surprise to me that a static instance of DateTimeFormatter performed roughly the same as a ThreadLocal<SimpleDateFormat> in the tests we ran that last time I was near this particular argument.

The vacuity of our example is, sadly, getting in the way of a more serious discussion. Let’s once again call upon our (now collective) imagination and enter a world in which we really do care about minimizing the resource usage of a call to format, or, at least, putting a constant limit on it.

A little deeper still

SimpleDateFormat (SDF) uses a buffer to build up the eventually returned String date, and it is the usage of this buffer that is at the heart of the lack of thread safety. We at least know that we have one buffer per instance of SDF though, and one guesses that we have one buffer per invocation of format in the JodaTime equivalents. Perhaps we see that we’re spending most of our GC time on these objects and wish to ameliorate affairs (although how these objects are escaping young gen to cause such problems seems odd).

Once again, we can very easily solve this by using a ThreadLocal<SDF> member in the class that needs it. But this isn’t the right thing to do. We’re introducing a synchronization (really? -Ed) mechanism at the base of our program. In general it is reasonable to be suspicious of newing business objects (other than temporaries or state) at runtime, particularly at the bottom of the stack.

Imagine you’re a thread executing this particular servlet; let’s say in response to a get request for some user details. The thread walks (I imagine the thread as a spider, here) across a series of data structures, picking out the pieces of data it requires, and then attempts to format the data according to some presentation code, and bzzt secretly accesses just the right formatter to do it.

Why is this bad?

unit < library < thread < application (the precise definition of what ‘<‘ means is left as an exercise to the reader)

Hopefully the buzzer going off points to where my discomfort comes from. Full marks if you recognised that it’s pulling out the SDF from the hat of the current thread’s context. Units, libraries – whatever term you prefer – ideally should not know too much about the context in which they execute. Having threading related knowledge in the wrong context is going to make your application hard to parse, but for the runtime and other programmers in the nearby.

Instead, we should push formatter construction up into the ‘thread’ area of our application’s wiring. That way the code below there stops knowing about its execution context, and we’re a little closer to our ideal ‘nested’ architecture, where our libraries are happy little islands that just know about other library interfaces and very little about threads.

This feels more maintainable than the ThreadLocal solution; consider what happens when a second ThreadLocal in a different servlet is required (yes, we could pass the ThreadLocal around, but if you’re going to do that work, why not do it properly?). In the more explicit approach, we just pass the formatter around a bit more, or if it’s a different class, our Thread object has another field, and all of our non-thread-safe components can be found in a single place by those who are unfortunate enough to follow us.

Apologies

I’ve stretched this example to its limit; I’d probably have opted for the Joda option. If you’ve made it this far, I congratulate you on your powers of imagination.

Oh, and, as ever, having written this, I find someone’s already written it better, years ago. Here’s Bob Martin espousing on ThreadLocal in a similar vain.

The Evil Of ToString

To the tune of Iron Maiden’s The Evil That Me Do.

I keep coming across this (anti)pattern, and this time it annoyed me enough that I wanted to write in more detail about it.

Object’s contract

Let’s kick off by examining the documentation of Object’s default toString(). N.B The docs on Object are actually very good; I recommend reading the whole lot while you’re there.

    /**
     * Returns a string representation of the object. In general, the 
     * <code>toString</code> method returns a string that 
     * "textually represents" this object. The result should 
     * be a concise but informative representation that is easy for a 
     * person to read.
     * It is recommended that all subclasses override this method.
     * 
     * The <code>toString</code> method for class <code>Object</code> 
     * returns a string consisting of the name of the class of which the 
     * object is an instance, the at-sign character `<code>@</code>', and 
     * the unsigned hexadecimal representation of the hash code of the 
     * object. In other words, this method returns a string equal to the 
     * value of:
     *
     * getClass().getName() + '@' + Integer.toHexString(hashCode())
     *
     * @return  a string representation of the object.
     */
    public String toString() {
        return getClass().getName() + "@" + Integer.toHexString(hashCode());
    }

So, a few interesting points to mention in there. First off:

“Returns a string representation of the object.”

This is the contract of toString : you call it, you get something that represents the object you’re calling it on. Debuggers make use of this by implicitly toStringing variables in the current scope, and it’s very helpful. Presumably that’s why we then get the later advice: “It is recommended that all subclasses override this method.”

An abuse of toString

Now you know about toString, let’s examine a particular abuse of it.

final String requestBody =
      new JsonMessageBuilder("heartbeat")
        .addValue(NodeName.TOKEN, "PC Nick Rowan").toString();

Here, toString is used in the StringBuilder/StringBuffer style; i.e toString is a synonym for build. Two (rhetorical) questions

  • Does that fulfil Object‘s contract?
  • Is there a name that works better?

Or, both questions combined: Why isn’t that method called build?

Other problems

Instincts aside, it turns out that this overriding of toString also creates some difficulties across a codebase in general. It’s difficult to tell at which points we want the built String for a business reason or for debug output. Plenty of libraries allow you to get into the bad habit of passing them an object that they implicitly toString (I’m looking at you, log4j).

Try a ‘find usages’ on an overridden toString method in your preferred IDE. Good luck working out which of those occurrences actually involve your object; particularly if the object in question implements one or more interfaces.

Theoretical alternative

If this were any other method, we might be suspicious that a concrete override was at the heart of the design. How could we change java to ameliorate this?

Well, the default implementation isn’t that useful. It just tells us the class name and the hashCode of our instance. We can probably lose this method completely, both pieces of information are available separately. We can discuss the problems with hashCode (more concrete overrides, odd name for default implementation) another day.

If we want to let the debugger know about a Stringy description of ourselves, why not be explicit, and use something like hamcrest’s SelfDescribing?

Side note: the implementations of appendValue in Description allow passing in an arbitrary Object in. Imagine my disgust.

Conclusion

Don’t override toString to provide business logic.

Corollary: alarms should sound as soon as you start writing tests for a class’s toString. Move that functionality to a method that better describes the behaviour you want.

This post was brought to you by the following reasonably reliable general principles