Code Craft
The art, science and craft of writing quality software

Aug. 16, 2005 - Blame the victim

Posted in Programming

Today I would like to walk in to a minefield with full knowledge that there may be an explosion.  I´ve decided that the NullPointerException (NPE) is a good thing. 

 

To understand what I´m talking about it is important to be aware of the distinction between Checked and Runtime exception.  Let me briefly describe Checked Exceptions and how they differ from Runtime Exceptions (which are exceptions as understood by every other language since CLU).  If you are unfamiliar with Exceptions in general, uhhm, well just look it up.  Checked Exceptions are, essentially, a way for the compiler to manage exceptions as an enforced contract.  The contract specifies that exceptions A, B and C can be thrown by a method and no others.  Every method must then explicitly define its contract about what it will and will not throw (either directly or indirectly).  The compiler makes no effort to ensure that runtime exception are declared or caught.

 

On the whole, I view Checked Exceptions as a useful feature that is massively over-used.  Many Java developers, on the other hand, have been converted to the school of thought that all exceptions thrown by their applications should be Checked and that the handful of unchecked (runtime) exceptions thrown by the system should be religiously caught and wrapped in checked exceptions. 

 

Since the dreaded NPE is a runtime error, trying to avoid it can end up creating lovely code like this:

public class LoveBoat

{

    private String captainsName = null;

 

    ...MISC CODE including a setter for captainsName...

 

    public String getRoster() throws BadCaptainException {

        if (captainsName == null) throw new BadCaptainException("...");

        return captainsName + " some other crew ";

    }

}

 

public class BoatManager

{

    private LoveBoat loveBoat = new LoveBoat();

  

    ...MISC CODE including a constructor that calls setCaptainsName()...

 

    public String getCurrentLoveBoatRoster() {

        String rtn = "";

        try {

            rtn = loveBoat.getRoster();

        }

        catch (BadCaptainException x) {

            // this can´t happen lets eat the exception to suppress it

            // since otherwise our contract would be wrong

            logger.error("AHHH, we should not be here.", stackDump());

        }

        return rtn;

    }

}   

This code is bloated and ugly and all because the BoatManager (who has carefully ensured that his owned LoveBoat was properly initialized) has been forced to catch and eat the BadCaptainException.  If the BoatManager passes on the exceptions (declares it as a possibility via throws) it is compounding sin by declaring exceptions it really can´t throw (although the compiler can´t know this).  Every user of the LoveBoat class will be forced to deal with an unthrowable exception. 

 

For those who think that having getCurrentLoveBoatRoster throw the BoatManagerException is the right solution, it´s now clear that you really do like runtime exceptions since you have turned your checked exception in to a runtime exception that you like to type a lot.

 

For those who think that having getCurrentLoveBoatRoster throw a new exception like RosterGenerationException I can only ask, "where will this madness end?"

 

If the set method had been replaced with a constructor the problem would have gone away (since the LoveBoat class could then be certain it was properly initialized before use).  If we had suitable defaults for all values (like "Stuebing" for the captain's name) the problem could also be avoided.  Unfortunately, for a host of reasons, we cannot always do either of these two things.  As a result some classes possess a "potentially" uninitialized state.

 

If we rethink our ideas about the dreaded NPE and drop all the exception code from LoveBoat.getRoster() and BoatManager.getCurrentLoveBoatRoster()  we end up with two trivial one-line functions. This is easier to read and better code.  In the original code there was the problem described earlier of future changes to the BoatManager class.  With the new implementation we are throwing in all the same cases, and people are welcome to catch those exceptions, they are just not required to. 

 

The core problem seems to be that the code is trying to catch programming errors in the wrong place and blaming the victim.  The cause of the error is "constructor" logic in bad code somewhere.  The caller of the LoveBoat.getRoster() method is the victim of that (potential) programming error, but it is still forced to take the blame by doing the work to manage it.

 

Because the compiler doesn´t (and can´t) know if the class has been constructed properly we have polluted our method signature.  The result is that the code is forced to detect the programming error at run time, but because run time exceptions are a "bad thing" the complexity is spread to where it is not useful.

 

For those who have accepted the argument that the exception really should be a runtime one but believe that we should throw something more "specific" instead of an NPE; fine.  The final exception will bubble up to the same place with roughly the same results, but your diligence is laudable. 

 

None of this implies that I think we should not try to avoid NPEs.  We should, but there is only one place where the work makes sense, and that´s at construction time.  If the class can it should ensure that it can never have invalid state.  Setters should most definitely check for that state (so that they can blame the culprit and not the victim), but allowing functions that use state to fail in a runtime way (even via NPE) is the best you can do.  Trying to do more at that stage in the lifecycle just makes matters worse.

 

As a parting note, the folks working on the Nice Programming Language (which is its own language but also can be viewed as an extension to Java) ensure a much higher degree of safety from these NPE problems by making the possibility of nullness something that is known up front (declaratively) by the language.  This will make things better, but like const in C++ they may find that people are forced to allow nullness in many cases to avoid the long-construction-cycle issue.  I´d love to hear from anyone with practical experience to confirm or refute this appraisal.

Post A Comment!

Aug. 31, 2005 - the opposite way

Posted by Anonymous
Interresting little essay :)

I find it funny to read how other and often more experienced developers rant about other developers practices, and imagine where I would fit in.

I personally prefer to only catch NPEs if the null value doesn't matter, and instead try to unittest (or just test) a programs ordinary and intended function - if I get an NPE, it means I've forgot something, and that something is most often a missing ArrayList or HashMap initiali-sumin-sumin aka. as a missing " fieldN = new MyClass()" in the constructors ;)

I would guess that catching NPEs is rarely part of the business "logic" - if you get what I mean by that. (if you think that "neither are HashMaps", then you didn't get what I meant ;)
Permanent Link

Sep. 2, 2005 - hmm

Posted by codecraft
I'm not sure I completely get your point, but in general you appear to be taking the least code approach. I agree with it in general, but do favor catching inbound bad initialization since the resulting stack trace tells you WHEN the problem happened. If you wait until the system uses the code it takes more time to debug since you can't be sure when it was created.
Permanent Link

Share and enjoy
  • Digg
  • del.icio.us
  • DZone
  • Netvouz
  • NewsVine
  • Reddit
  • Slashdot
  • StumbleUpon
  • Technorati
  • YahooMyWeb
<- Last PageNext Page ->

Kevin Barnes

Code Craft is the place for my thoughts, rants, ideas and occassional jokes on what it means to write code, why some people are better at it than others, and how we think about software in general.

Links

Home
View my profile
Site Feed (RSS)
Archives
Email Me
StorePerform
TechNexxus
Soul Craft (my stories and discussions)
My Wall

Joel on Software
Paul Graham
Patric Logan
Martin Fowler
Free Blogs
Free Blog



Copyright (C) 2005, Kevin Barnes. All rights reserved.

portfolio