Stephen on Software aka SOS

July 15, 2009

Bad Code – Bad API, Bad Configuration

Filed under: bad code — Tags: — sljm @ 2:00 pm

Was trying to use an library/jar file in my project today. The jar was a wrap around to call a webservice (lets call the interface Service and the implementation ServiceImpl), the code has interface and all that and even had a factory class (ServiceFactory) built in to initialize the Service for you. There was no source code, I only had the class files, so i browsed the classes through NetBeans to see how to best use it.

I am a big fan of dependency injection, I thought i could just use ServiceImpl to initialize the service and inject into the classes that need it, but there was something wrong. Look at the code below.

public class ServiceImpl{
private ServiceImpl(){}

public String methodOne(String string){}
public String methodTwo(String string){}
}

Did you see the problem? The constructor is marked private! There is no way to initialize the class! Except through ServiceFactory. Take note that this is suppose to be a class that calls a webservice, so I am still looking for a place where to put in the webservice url.

When I went to another colleague to ask how do I use it, he showed me sample code below.

Service s=ServiceFactory.getInstance();
s.methodONe(someString);

I asked how do I set the URL for the webservice? He told me the one (and only way) was to use the configuration file in the jar itself.

These are the problems as i see it.

  1. ServiceImpl constructor is private. That means I can’ test it by constructing a new object., i can’t inject it into my code since the only way to construct it is through ServiceFactory. I can’t mock it too since everything is done in the factory. This is not good, hard to test and hard to mock.
  2. There is only one way to configure the Service and that is through ServiceFactory and the config file. That means I can’t programatically configure it through code, that makes testing hard, that makes using other forms of configuration hard (like you own custom config file, Spring applicationContext and so on).  Singletons are Evil use DI frameworks if you can.
  3. The API is unclear, from the api you can’t really tell what is needed to use it. Except that you know you need to get it through a Factory. But since all the confguration and setting up is done in the factory, you can’t tell by looking at the API what is needed to set it up.

The last point is the main one i guess, the API is not clear, we can’t tell how to use except though some internal knowledge. It didn’t help that there were no javadocs, some of the parameters were names arg0, arg1, string1 (you get the drift).

For people who are writing apis for other people to use, perhaps take a look at Spring and Hibernate’s configuration design.

In short, if you allow your objects to be instantiated programatically , you allow the possibility of using properties files, xml files or custom files, look at Spring and Hibernate. This is especially important when writing apis/libraries that are going to be used by different projects.

Make the apis clear, if it needs a URL put the need somewhere in the constructor of the class. So that people know what is needed to instantiate it, the api can be the documentation especially with modern IDE like Eclipse and NetBeans.

Some links

Singletons are Evil

Singletons are not Evil

Blog at WordPress.com.