I almost never give my opinion about code I see, but I felt this one was interesting and educative. It all started while working on a legacy project (an ASP.NET web application), I found out that most connections were not closed. So I fixed them all and making sure every connection was closed once the I finished with it, should an error occurred or not.

try
{
    using (IDataReader reader = SqlHelper.ExecuteReader(Sql.MainConnection, "sp_name", paramter1, parameter2))
    {
        while (dr.Read())
        {
            //do something
        }
    }
}
finally
{
    Sql.MainConnection.Close();
}

Pretty basic stuff. Today I deployed the application to a server for some testing. After two or  three users getting in, Boumm! SQL Server is dead. I check the activity monitor and found out that a hundred connections were still open. The reason is : I have wrongly assumed that a static class in the project was providing me with a connection that was shared between all data access class.

I was sure I was closing all the connection, but I ended up closing nothing.  Let’s take a look at the MainConnection property of the Sql class :

public static SqlConnection GetMainConnection
{
    get { return new SqlConnection("connectionstring"); }
}

Each time I was accessing the database, and it was accessed a lot (20 times per request), I ended up with a lot of open connections. Enough to make SQL Server unavailable for some time. You could argue that it was my mistake and I shouldn’t be assuming things like this. But in my opinion, it’s a case of bad naming. The choice of name for the property that return me a connection was inconstant. If I access a property, I assumed that if I call it five times in a row, I would have five times the same result. The best choice would have been to create a method that return a new connection and name it accordingly.

public void SqlConnection GetNewSqlConnection()
{
    return new SqlConnection("connectionString");
}

It would have reduced the ambiguity and would have been a lot more easy to read. Good naming is very important and can greatly enhance productivity on a project. Good code should be easy to read and you should not ask yourself if a calling a method would cause side-effects or not doing what it said it should.