Downcasting is a Code Smell

Before you go any farther, a Code Smell simply implies that there *might* be something wrong with your code and that you *definitely* need to evaluate your code. 

 

Downcasting, in my opinion, is a code smell.  Specifically, I think code like this below is smelly: 

        public void Filter(IFilter filter, object[] targets)

        {

            if (filter is StringValueFilter)

            {

                StringValueFilter stringValueFilter = (StringValueFilter) filter;

                // call a specific member on the StringValueFilter

            }

        }

There’s a couple different problems that the downcasting smell might be exposing:

  • Your abstraction is leaky and needs to be refactored or eliminated.  In the case above, everything should be accomplished through the IFilter interface.
  • You’ve violated Tell, Don’t Ask, which is another way of saying that you’re not properly encapsulating functionality.  Encapsulation is important for a couple reasons.  First, encapsulation makes code easier to use because there is less stuff necessary to know or do to make it work.  Secondly, poor encapsulation can greatly increase duplication throughout your code base.

To avoid downcasting, you might take a look at:

  • The Information Expert pattern from Craig Larman, or
  • Use Double Dispatch.  Double Dispatch used to feel really odd to me, but I’ve used it in a couple places in the last year where it has streamlined quite a bit of hacky if/then logic.

About Jeremy Miller

Jeremy is the Chief Software Architect at Dovetail Software, the coolest ISV in Austin. Jeremy began his IT career writing "Shadow IT" applications to automate his engineering documentation, then wandered into software development because it looked like more fun. Jeremy is the author of the open source StructureMap tool for Dependency Injection with .Net, StoryTeller for supercharged acceptance testing in .Net, and one of the principal developers behind FubuMVC. Jeremy's thoughts on all things software can be found at The Shade Tree Developer at http://codebetter.com/jeremymiller.
This entry was posted in Uncategorized. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://codebetter.com/blogs/jeremy.miller jmiller

    Travis,

    That was a pretty off the cuff example, but let’s try. All you really want to do is have that Filter(IFilter) method work by calling a member on the abstracted IFilter interface instance that is being passed in. It might be something like IFilter.Matches(target) returning a boolean, or with double dispatch it would be something like filter.Filter(this);

    I’ve got an idea for a post to demonstrate a Visitor pattern that would demonstrate double dispatch.

    Two years ago I wrote a business rules engine that worked on data in DataSet’s. I had an IFilter interface that could test conditions on the rows coming in to the engine. Some of the IFilter’s could just declaratively use little sql snippets for the condition, while others had to programmatically check each DataRow. A few did both. I simply made the rules processor class strictly use the IFilter interface to add SQL snippets and then later to test the DataRow’s programmatically without knowing or caring how each IFilter instance worked.

  • http://www.cycleutah.com Travis

    Jeremy, could you rewrite that code the “correct” way? I’m really just curious.

  • http://codebetter.com/blogs/jeremy.miller jmiller

    Scott,

    The code that may or may not have inspired this post wasn’t VB. I don’ t know why you automatically assumed that bad code was from VB6 developers;)

    It’s still better than the “legacy” code Steve and I wrestled with in Austin.

  • http://www.bellware.net ScottBellware

    Dude! I didn’t realize that you were leaving Austin for New York to get the opportunity to maintain headless VB 6 code. Heck, you coulda come to work for me maintaining our legacy apps. Har-har-har!

    Just kidding. Keep up the good fight. The rebel would be diminished without people like you.