Sunday, January 16, 2005

Useless overload

Gotcha! — That's how I felt. I changed code like this:

final Worker worker = new Worker();
final Implement hoe = new Hoe();

worker.work((Hoe) hoe);

To remove the "obviously" useless cast. Well, no, not useless at all. I had overlooked Worker.work():

public class Worker {
    public void work(final Implement o) {
        // Generic work
    }

    public void work(final Hoe hoe) {
        // First do generic work
        work((Implement) hoe);
        // Now do hoe-specific work
        hoe.work();
    }
}

Argh! Of course, there are several much better ways to do this, and I could have changed the declaration of hoe instead of removing the cast. Putting the onus on the caller to pass in the right type of object is just grotesque. And by removing the cast, I broke the code.

I congratulate whomever devised that nasty trap, requiring two useless casts to work correctly and splitting them between the caller and the method no less! Unfortunately, it was purely accidental on their part.

Next time, please code this:

public interface Implement {
    public void work();
}

public class Worker {
    public void work(final Implement implement) {
        // First, generic work
        // Lastly, implement-specific work
        implement.work();
    }
}

Or, if you prefer, make Implement a class with a default, empty work() method. Just something that avoids uselessly useful casts and that requires ESP from the caller.

No comments: