Legacy Wednesday: Kill Me Now

Posted by Tom on 2014-01-29 20:23

If I ever meet the person who wrote our old database connection code I'm going to hit them in the crotch with a cactus.

/// <summary>
/// Checks the connection is closed 
/// </summary>
void IDisposable.Dispose()
{
#if (DEBUG)
    if (_connection != null)
        Debug.Assert(_connection.State == ConnectionState.Closed, 
            "**** WARNING **** Connection not closed by " + _constructorInfo, 
            "Full Stack trace in Constructor: " + _constructorInfoFull +
            "***** IF YOU ARE SEEING THIS MESSAGE ON A PRODUCTION SERVER then some " +
            "numpty built the framework in DEBUG - please rebuild in RELEASE *****");
#endif
}

For those who don't .NET much: implementing the IDisposable interface (and by association the Dispose method) is designed to give you a place to mop up unmanaged resources so they're not leaked when your object goes out of scope and the garbage collector comes and reclaims your memory. It also allows you to use the class in language constructs such as the using statement to do it automatically. This is a totally legitimate pattern in .NET development. That is literally what it's there for.

What it's not for is mocking the developer who uses your class and then leaking the resource anyway. But only if they're using it correctly. Turns out nothing in the codebase ever calls this class' Dispose or wraps it in a using block, because if you do it crashes your IIS app pool.