Friday, February 8, 2008

How To Quickly Frustrate Future Developers

Simply put, if you want to frustrate your teammates, let your abstractions leak.

Have a production implementation of an interface with stubbed calls? How about a subclass that neuters parent-class functionality? If you do, you may be well on your way to setting a trap for future developers (quite possibly your future-self).

Interfaces
If you really don't need all the methods on an interface, then the interface is too big and should be broken out. Here's a contrived example. Consider what an IDaddy interface might look like:


interface IDaddy {
public void changeDiaper(Baby baby);
public void procreate(IMommy mommy);
//More methods, but significantly less than the IMommy interface.
}

This interface had better have a singleton production interface. Otherwise, other implementations of IDaddy that implement the procreate(IMommy) will have some serious 'splaining to do! (IMommy better be a singleton as well, or the IDaddy is in big trouble...).

Anyway, there are certainly uses for having someone other than Daddy change the baby diaper. Perhaps a better set of interfaces would be:

interface IBabySitter {
public void changeDiaper(Baby baby);
}

interface IDaddy extends IBabySitter {
public void procreate(IMommy mommy);
}

Now we can have several implementations that can change the baby diaper without creating family strife!

Subclasses
Just ran into this class today:

public class UnknownColumnNameException extends RuntimeException {

private static final long serialVersionUID = 1L;

String strColumnName;

public UnknownColumnNameException(String colName) {
strColumnName = colName;
}

public String toString() {
return super.toString() + " ColumnName = " + strColumnName;
}

public void printStackTrace() {
System.err.println(" ColumnName = " + strColumnName);
super.printStackTrace();
}

public String getColumnName() {
return strColumnName;
}

For those of you not completely familiar with Java's exceptions, you may not notice what is missing. In the top level processing, we have a generic, last-ditch catch of all Throwable so that we can dump log files, etc. This exception has not set any message. So, when I do an ex.getMessage() in the top-level catch, all I get is null. Thanks for nuthin'.

The user has to know about this specific exception type in order to get the useful information back out of it. Bad, Bad, Bad! If this pattern was extended, then we'd have to put a catch for each type of exception that just might happen, and pull the specialized information out of each one. Forget it!

The fix? Simply set the message in the subclass. In this case, this can be done by leaning on the superclass constructor as follows:

public class UnknownColumnNameException extends RuntimeException {
...
public UnknownColumnNameException(String colName) {
super("Could not find column with name '" + colName + "'.");
strColumnName = colName;
}


Simple fix. Great improvement in usability. Perhaps next time, I won't have to resort to the debugger to know that the input file has a problem.

What kind of abstraction leaks have you found?

Tuesday, February 5, 2008

How Can a Simple Code Template Change Make Life Better?

Ever felt like making a small change has sent you down a rabbit hole? I sure have. Consider the following classes:


public interface NotificationService {
void sendNotification(String user);
}

public class Unit {
private NotificationService notification;

public Unit(NotificationService notification) {
this.notification = notification;
}

public void doSomething(String username) {
notification.sendNotification(username);
}
}


You might write a test to ensure that Unit.doSomething() passes off the username to the NotificationService.sendNotification(). This would be a very simple mock test.

Now, let's assume that everything shown so far is already under test, 100% coverage. Requirements have changed, and now when we doSomething(), we should check for pending notifications for that username first.

The first change to make would be adding a method to the NotificationService interface:


public interface NotificationService {
void sendNotification(String user);
Notifications retrievePendingNotifications(String user);
}


Now, we can write a test for the Unit class, using a mock of NotificationService to verify that doSomething() will check for the pending notificiations. Doing Test-Driven Design, we want to make sure that we are focusing on changing one class at a time. However, now that we have added retrievePendingNotifications() to the interface without actually implementing that method on the implementing classes, we will have a compile error.

One choice for getting around this would be to go down the rabbit trail, writing tests for the implementing classes and actually implementing the new method. Personally, I don't like to do this because it changes my focus. I would rather first ensure that the caller is correct before worrying about the actual implementation of the interface.

Most modern IDE's will offer a quick-fix to stub out the implementation of the interface so that things will compile. The default behavior for this default implementation is typically to return the simplest thing possible. If the function is void, then the stubbed method does nothing. If the function returns a primitive, say int, then the IDE will put in something like return 0; to make things compile.

This kind of default stub has already worried me that I will forget to actually do the implementation. In fact, I have done just that before, sometimes not finding the hole until a day or two later. At that point, figuring things out sometimes involves going to the debugger to figure out why I'm getting a NullPointerException.

Enter code templates. The default implementation can be modified in most IDE's. Here is a sample of how I changed my code template to prevent this problem in the future:


public Notifications retrievePendingNotifications(String user) {
throw new NotImplementedException(
"NotificationServiceImpl.retrievePendingNotifications() not yet implemented.");
}


Now, I don't have to try and keep that little piece of information in my head. I can rest easy that, if I forget to implement something, it will still fail (hard) during integration testing. Now, I will know the direct cause of the problem immediately.

It's amazing to me how this simple change has made me rest so much easier.

Go forth and write tests!