Casts are a code smell

Posted on by

Explicit casts. I wish there was a compiler option that could make it flag these as warnings (or even better, errors). These hateful things sit there like timebombs in your code, just waiting for an opportunity to explode in a shower of ClassCastExceptions. They are at their most potent when buried somewhere in an infrequently used portion of the application, sometimes waiting for days or weeks until an unsuspecting tester (if you’re lucky) or user (if you’re not) stumbles across them.

They’re often not easy to get rid of. The approach I take is to try and extract out all the places something is being cast into a single method, and funnel all the calling code through that. Once that is done it gets easier to see how to remove the cast completely.

Added later: Obviously its impossible to entirely remove casts when dealing with things like collections. The best you can shoot for is exactly one cast per collection access, by wrapping up your collection accessor calls inside more type-specific methods.

3 thoughts on “Casts are a code smell”

  1. Unfortunately we obviously have to get some of our reuse out of downcasting (like with Iterator.next)…however when generics come along, downcasting will be an order of magnitude smellier.

  3. I will sometimes encapsulate a cast in a method, especially if I need to do it all the time. The Java compiler will automatically inline the method, and it makes it super easy to manage the code. You might be extending a class that has a method getX() that returns a Collection, for instance, but you want to downcast that Collection to a List every time. So you’d simply implement a method called getXAsList().

    Important: say you’re providing your own implementation of getX() in the subclass. In this case, you’d override getX() and implement the method body as if you were not going to even write getXAsList(). This ensures that you do not violate the contract that is binding between subclasses and their parents when performing an override. In fact, whether you override the method or not, the body of getXAsList() should always be:

    public List getXAsList() {
    return (List) this.getX();
    }

    sev

Comments are closed.