Spot the Bug - Using Statement Part 1

Several months ago, I was reviewing some C# code of one of my teammates when I realized there was a subtle bug in the program. The bug was not easy to see right away. I realized that C# had certain constructs in place to make it difficult to create bugs like this. These constructs are great, but they mask certain design principles and make it possible to learn programming without learning the underlying design principles. I reviewed the bug in question with my team during a code review and made it kind of a game to find the bug and talk about the underlying design principles.

I did a talk titled "Spot the Bug" for the Code PaLOUsa conference last week. The talk was geared for two primary audiences. The first audience is developers that are earlier in their career. This audience benefited from this talk in seeing common coding mistakes and learning some common design principles. The other audience for this talk was developers later in their career or developers in positions to mentor others. This audience was shown an approach for highlighting some of these common mistakes and seeing ways to address them with their creators.

This is the first topic that I covered in the talk.

Below is a class to access a database. It has methods to open and close the database connection as well as a method to work with the data. Can you spot the bug?

Most C# developers are looking at the code above and thinking to themselves, "Why didn't he do this with a using statement?" These developers are absolutely right. The bug in the code above is that the DataGatherer class separates the responsibility of managing the database connection between two different methods, effectively forcing the developer that uses this class to be responsible for doing things correctly.

The using block in C# forces a developer to keep the responsibility of opening and closing database connections in the same code block, mostly ensuring that the right things are done in regards to the database connection. (It is still possible for a developer to forget to close the database connection within a using block, but the using block will force the connection closed and dispose of the connection. It just may take the garbage collector longer to get to it.)

The better code is below:

You'll notice in the better code that there is a lot less code! Keeping the opening and closing responsibilities together inside the using block, reduces the amount of code in the class and creates a much better program.

This subtle bug is easy to overlook if you learned to work with a database after the creation of the using construct. The using construct is a way to talk to new developers about class and method responsibilities and the need to keep similarly oriented responsibilities together.