The Lurker

Don't initialise it yet

Today's lesson is inspired by generated code produced by an EJB container which shall remain nameless:

PersistenceManager pm = null;
try
{
    pm = findPersistenceManager();
    pm.fetch(something);
}
catch (PersistenceException e)
{
    throw new ObjectNotFoundException(message);
}
finally
{
    pm.close();
}

Needless to say, findPersistenceManager() throws PersistenceException if you have not configured persistence management correctly, leaving pm set to null. So that finally block triggers a completely unhelpful NullPointerException.

Also needless to say, this makes it difficult to identify configuration errors.

The moral of the story?

1. Don't try to call close() on something you may or may not have acquired.
2. Don't initialize pm until you've successfully located the PersistenceManager; that way, the compiler can tell you if you try to do #1.
3. It won't kill you to nest a try block inside another try block. The correct thing to do here would have been something more like:

PersistenceManager pm;
try
{
    pm = findPersistenceManager();
    try
    {
        pm.fetch(something);
    }
    finally
    {
        pm.close();
    }
}
catch (PersistenceException e)
{
    throw new ObjectNotFoundException(message);
}

Interestingly, Python supports try-finally and try-except (except being equivalent to Java's catch) but not try-except-finally blocks, which results in code similar to the above for ensuring that a resource is released correctly.

Personally, I prefer that approach. It would be OK to test (pm != null) before calling close(), but I prefer the above construct, even though it is a bit noisier, because it allows the compiler to help you catch this type of error.

Related topics: Python Rants Java