Bad API and worse coding practices

Writting a good public API is a very hard job. The API exposes some of the system's functionality and if you plan on releasing your software in the future several times, you better spend a good portion of your time on the design of the public interfaces to your system.

One of the reasons is that you do not want your API to change over time. A single change would make the new release incompatible with the previous ones and all the third-party code that was written and worked well needs to be fixed, before it can work again with your latest shiniest version.

In the past, I worked on a project where I was faced with a custom API. And let me tell you, the interface was far from ideal. Not because it changed over time, but because it was not designed well (or designed at all?) Probably designed by street-side programmers who not only exposed the API via abstract classes that you had to extend (there is this thing called Interface in Java) but also having concrete classes in the method signatures (interfaces anyone?).

For example a method looked like this

public Vector getGetNamesFromContacts(Vector contacts)...

So not only you cannot use your collection of choice but you have to use Vector. I ask you, why Vector? We all know how slow they are when compared with unsynchronized lists (e.g. ArrayList). I don't really think that the synchonization was necessary.

Anyway, imagine that you are given the following abstract class that you can extend. Remember, the API is really bad and you have no access to the Module interface. In fact, there may not even be such an interface. The only thing that is exposed to the outside world is the abstract AbstractModuleImpl class.

There are three methods in AbstractModuleImpl class: execute(), setUp() and cleanUp(), all are public, execute is also abstract and let's say that their signatures do not really matter at this time.

These methods are given and you can implement them in order to get the set-up before work, actual work and clean-up after work done.

In the next step, we implement our own class. This class is named ViewModule and extends the given abstract AbstractModuleImpl class. As the super class is abstract and our class is concrete (meaning not abstract) we need to implement all methods that were defined as abstract - execute().

We also added and implemented two protected methods doTheThing() and doSomeExtra(). These methods are called from inside execute() method.

Later we also wanted to implement EditModule class. This class shares 90% of the code similarity with ViewModule. Naturally, that would be best implemented through inheritance. The base class would implement the common methods and then the concrete classes with varied functionality would be implemented as its sub-classes.

I wrote about simplicity of the design in extreme programming in my blog entry Simplicity and XP.

In our case we leave ViewModule as is and extend from it. As you can see from the class diagram EditModule class extends ViewModule class. It overrides execute() and doTheThing() methods. It does not override doSomeExtra() as this method is re-used as is.

Everything looks quite fine, right? But here comes the twist! One of the respected street-side programmers (who does not used Iterators and uses Vectors for everything he codes, just because the other API designed did) in that company told me that this approach would not work. The reason being that it would only work when our implementation class directly extends AbstractModuleImpl. He tried it before, and it did not work. I do not know what he tried, but did not want to qustion his judgement. I just took it as a fact. But still... why would anyone impose such ridiculous limitation on public API?

Anyway I proposed the following design. It was a bit more complicated, worked around the limitation of the API and still reused most of the code.

In this case ViewModule2 and EditModule2 share the only similarity, which is they call work() method on their associated command objects.

In this way I could still implement 90% of the common code in ViewCommand class and reuse it in EditCommand class. Also ViewModule2 and EditModule2 classes directly extended AbstractModuleImpl as was required.

Anyway, despite the effort, the code changes were not understood by the street-side programmers (each module should be implemented as one class so it can be delivered stand-alone) and when I returned to work on Monday the code was reverted back to original implementation before my changes and EditModule class was implemented by deadly copy-paste-modify operation (ZERO reusability) based on the code of ViewModule class.