Never implement doHickey

I sometimes feel like an impostor when I talk about code design because I have absolutely no formal education in it, and I’ve been imprinted with the attitude that it’s the algorithms, their correctness and efficiency, that you’re supposed to worry about. Working on larger projects, in companies and on my own, I’ve come to realize that code design is important, difficult, and interesting, but I’ve learned it almost exclusively from experience.

Recently, I noted that I seem to have walked into the same trap many times in making my code conform to a framework, and recognized how to get out of it. Could I have avoided it if I had some code design in my education? I don’t know, but given that I had the realization after almost 40 years of programming, I don’t think it’s entirely trivial to everybody else.

I’m formulating the scenario as generally as possible using building metaphors, but taking syntax and language elements, like class and interface, from Java. If you get what those mean, it should be applicable to other languages and paradigms as well.

The general scenario

Let’s say you need boxes to be built in your code, using a hammer. You look around, and find that the environment you’re working in has a framework for building stuff, which you decide to use. There’s a class Builder that has a method

void buildBoxWith(Tool)

(where Tool is a quite general interface) and another method

Tool getHammer()

So if builder is an object of the Builder class you just need to write:

Tool hammer = builder.getHammer();
builder.buildBoxWith(hammer);

It works, fine. But later you need your program to build a different kind of box, so you decide to create your own class BoxBuilder with a different box building method. Maybe you make BoxBuilder a subclass of Builder or maybe you let it share some common interface, it doesn’t matter for the example, but you give BoxBuilder a method buildWith that is analogous to Builder.buildBoxWith:

void buildWith(Tool)

It turns out to be slightly awkward to write this method with Tool as the parameter type, because Tool is designed to be able to represent very diverse kinds of tools, some of which are nothing like a hammer, but you find a way, maybe by glancing at the implementation of the standard buildBoxWith method. Then you use your BoxBuilder like this:

Tool hammer = builder.getHammer();
BoxBuilder bbuilder = new BoxBuilder();
bbuilder.buildWith(hammer);

So far so good.

Later again, you need to implement a special hammer to sometimes use with box building. But your box builder should still be able to use a standard hammer.

So you decide to write a SpecialHammer class, and have it implement the Tool interface so you can pass it to your BoxBuilder.buildWith method. Then it has to have all the methods of Tool, even those that aren’t relevant for a special hammer (or for any hammer). Some of the methods could get useful at some point, so you implement them in some sensible way, but some you can just leave empty (return null or something), and in some – the cutWood method for example – you can’t do anything else than throw UnsupportedOperationException. You grumble a bit about the bad design decision somebody made to force you to write a lot of nonsense, but you work on.

Then you get to a Tool method called doHickey, which according to the documentation should “perform hickey for the tool”. You’re not sure what that is, and at first you just write an empty method body, but then for some reason you come to think that maybe you should implement it in a sensible way. For instance, maybe the documentation of some more relevant method says that it should behave differently depending on whether hickey has been done or not. Should you read up on hickey and do the work to create an implementation that is as sensible as possible for a special hammer?

No. This, if not before, is where you should realize that you’re going down the wrong path. Do not implement doHickey! Stop, back up, and refactor. More precisely, you can do this:

Define your own interface Hammer, which is general enough to make sense for both the special hammer and the standard hammer (and other hammers) but don’t make it more general than that, don’t have it implement the general Tool interface, and let it have only the methods that you have any use for. Then rewrite BoxBuilder to work with the Hammer interface instead of Tool. Yes, it might mean that some of the work you put into making it work with Tool is wasted, but sentimentality is a bad attitude in programming.

Then you write your SpecialHammer class, an implementation of Hammer, which should be straightforward. And, to be able to use a standard hammer in BoxBuilder, you write an adapter, a wrapper that let’s it appear as a Hammer, something like this:

class StandardHammer implements Hammer {
    private final Tool stdHammer;
    StandardHammer(Tool t) {    // constructor
        if (!t.isHammer()) { throw new EveryToolIsNotAHammerException(); }
        this.t = t;
    }
    void swing() { t.doYourThing(); }
}

It may not be that simple, and you may find some use for the awkward code you just removed from BoxBuilder (happy?), but this is where it belongs.

To summarize: you don’t have to force your code to adapt to a framework interface designed for something else. Instead, you can create an interface that is suitable for your purpose, and integrate it with the framework using the adapter pattern or something else to wrap standard framework objects for use in your code. There’s no point in implementing irrelevant methods that nobody asked for.

The specific case

Before Klipspringer version 4, the interface for receiving audio output in the track player was SourceDataLine (which is a strange name, but in the Java audio system you send output to a “source” and receive input from a “target”). SourceDataLine objects were sent around as parameters and had to be dealt with by various parts of the code.

For the purpose of sending audio data to files or channels (for streaming for instance), I wrote a couple of classes that implemented SourceDataLine, and had to supply implementations of a bunch of methods that weren’t relevant to Klipspringer use. Also, SourceDataLine and TargetDataLine use byte array as the data transport format rather than ByteBuffer, which make them awkward to integrate with NIO. When I started with the direct ALSA module, the awkwardness became too apparent to ignore, and I realized that the inheritance structure should be turned inside out. I created my own interface hierarchy based on two interfaces I called PcmWriter and PcmReader and wrote an adapter to get SourceDataLine into my framework instead of the reverse.

The change had greater consequences than I had predicted: it made it possible to see how I could have organized other things better, which triggered a cascade of refactoring. I could take advantage of the fact that Klipspringer is a hobby project without any deadlines or backward compatibility requirements, and went ahead with making the code more satisfying and beautiful, despite the fact that it introduced very little new functionality. It took a lot of time and was great fun, and resulted in a code base where it was much easier to find and eliminate bugs. The changes ended up being so large that I decided to take the version number from 3.2 to 4.0 rather than 3.3.

Leave a comment

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.