I came across this little gem whilst reviewing a change to what was once a reasonably well implemented Singleton class:

    public static ResponseMapper getInstance()
            throws ConfigurationException
    {
        if (theInstance == null)
        {
            synchronized (theInstance)
            {
                theInstance = new ResponseMapper();
            }
        }
        return theInstance;
    }

Apart from the fact that synchronizing on null is obviously wrong (unless the class is designed specifically to throw NullPointerExceptions upon usage!), this class has an extensive JUnit test which proves that this change kills everything, but the developer didn't even run the test before checking in. OK, well at least our Continuous Integration (CI) environment will run the test for the developer, flagging the errors and emailing the developer with the results of the carnage they have caused... but do they check the CI emails, no of course they don't, that would be too easy...

Aaargh!

EdD