Quarantine the ADO.NET Contagion in your Code

From my perspective the .Net world is clearly becoming more sophisticated in our approach to persistence. Reusable persistence solutions like NHibernate or NEO are starting to become more popular, reducing the need or desire to work with ADO.NET directly.

However, most of the .Net systems out there that I encounter still have old fashioned data access layers rolled by hand. This doesn't have to be that bad, ADO.NET is pretty easy stuff, and the .Net community is generally pretty strong in data access. The problem is that ADO.NET usage has to be done right, every single time. Forget to close a connection or an IDataReader somewhere, then add in enough volume through the system, and watch your application roll over and crash because the database has too many open connections. The same thing with exception handling occurs. You have to have the same "try/catch/finally" code every single time you use ADO.NET code.

The second problem is a little more endemic. Because ADO.NET is so easy to use, developers coding a web page or a business project give into temptation to just write the data access code right where it is needed instead of separating out the data access concern with a little thought. These people are often colloquially known as "Mort's." To Mort, please stop doing this. Especially if you're writing code that I have to support. Just in case it's not obvious, writing the ADO.NET code straight into a business object basically eliminates any possibility of writing easy unit tests.

My advice is to simply count the number of times the following lines of code show up in your application. Offhand, I'd say the maximum number of times this code should be called to be no more than 3-5 times.

• connection.Open();
• command.ExecuteNonQuery();
• command.ExecuteReader();
• connection.Close();

You can eliminate a grotesque amount of duplicated code by simply centralizing the low level ADO.NET manipulation into one or very few classes. It's much easier to write the error handling and connection management once than it is to get it right everywhere. It'll also allow you to create a better answer for tracing and instrumentation. All other higher level data access layer classes call into the central execution class to actually execute database commands. Other than the possible exception of DataSet's, nothing from the System.Data namespace should ever leak out beyond the classes that are specific to data access.

Here's an example of writing the inner code execution class from our data access layer:

using System;
using System.Data;
using StructureMap.DataAccess.ExecutionStates;

namespace StructureMap.DataAccess
{
 [Pluggable("Default")]
 public class DataSession : IDataSession
 {
  private readonly IDatabaseEngine _database;
  private readonly ICommandFactory _factory;
  private readonly IExecutionState _defaultState;
  private readonly ITransactionalExecutionState _transactionalState;
  private IExecutionState _currentState;
  private readonly ICommandCollection _commands;
  private readonly ReaderSourceCollection _readerSources;

  [DefaultConstructor]
  public DataSession(IDatabaseEngine database)
   : this(database, 
   new CommandFactory(database),
   new AutoCommitExecutionState(database.GetConnection()),
   new TransactionalExecutionState(database.GetConnection()))
  {

  }

  /// 

  /// Testing constructor
  /// 
  /// 
  /// 
  /// 
  /// 
  public DataSession(
                    IDatabaseEngine database, 
                    ICommandFactory factory, 
                    IExecutionState defaultState, 
                    ITransactionalExecutionState transactionalState)
  {
   _database = database;
   _factory = factory;
   _defaultState = defaultState;
   _transactionalState = transactionalState;

   _currentState = _defaultState;

   _commands = new CommandCollection(this, _factory);
   _readerSources = new ReaderSourceCollection(this, _factory);
  }



  public int ExecuteCommand(IDbCommand command)
  {
   try
   {
    return _currentState.Execute(command);
   }
   catch (Exception ex)
   {
    throw new CommandFailureException(command, ex);
   }
  }

  public int ExecuteSql(string sql)
  {
   IDbCommand command = createCommand(sql);
   return this.ExecuteCommand(command);
  }

  private IDbCommand createCommand(string sql)
  {
   IDbCommand command = _database.GetCommand();
   command.CommandType = CommandType.Text;
   command.CommandText = sql;
   return command;
  }

  public IDataReader ExecuteReader(IDbCommand command)
  {
   try
   {
    return _currentState.ExecuteReader(command);
   }
   catch (Exception ex)
   {
    throw new CommandFailureException(command, ex);
   }
  }

  public IDataReader ExecuteReader(string sql)
  {
   IDbCommand command = createCommand(sql);
   return _currentState.ExecuteReader(command);
  }

  public ICommandCollection Commands
  {
   get { return _commands; }
  }

  public IReaderSourceCollection ReaderSources
  {
   get { return _readerSources; }
  }

  public void Initialize(IInitializable initializable)
  {
   initializable.Initialize(_database);
  }

 }
}

using System;
using System.Data;

namespace StructureMap.DataAccess.ExecutionStates
{
 public class AutoCommitExecutionState : IExecutionState
 {
  private readonly IDbConnection _connection;

  public AutoCommitExecutionState(IDbConnection connection)
  {
   _connection = connection;
  }

  public int Execute(IDbCommand command)
  {
   try
   {
    setupConnection(command);
    return command.ExecuteNonQuery();
   }
   finally
   {
    cleanupConnection(command);
   }
  }

  private void setupConnection(IDbCommand command)
  {
   command.Connection = _connection;
   _connection.Open();
  }

  private void cleanupConnection(IDbCommand command)
  {
   try
   {
    command.Connection = null;
    _connection.Close();
   }
   catch (Exception)
   {
   }
  }

  public IDataReader ExecuteReader(IDbCommand command)
  {
   try
   {
    setupConnection(command);
    return command.ExecuteReader
(CommandBehavior.CloseConnection);
   }
   catch (Exception)
   {
    cleanupConnection(command);
    throw;
   }
  }
 }



}

In this example, all data access classes have to use a class called DataSession
to execute an IDbCommand object or a sql string. The DataSession class delegates to an IExecutionState interface object. The AutoCommitExecutionState deals with all of the connection opening and shutting, as well as providing a consistent error handler to report the actual sql (with parameters) that failed. By centralizing this operation to one spot, we could add extra code to provide more instrumentation or simply change the error handling across the board. Most importantly, other classes don't have to be obfuscated by mucking with so much ADO.NET manipulation.

My Poor Friend

One of my previous career stops was at a very bad internal IT shop. The leadership in our division was making it painfully clear that developers were merely a commodity, and undeserving of anything like good working conditions, raises, or a career path. After years of abuse working on a truly horrendous VB6 system (inspired by the architectural writings of a "Super Mort"), a friend of mine named Scott secured a transfer to a different organization. Scott was baited by promises of getting to do hard core J2EE development, the hottest thing going at the time.

Upon arrival at the new digs, Scott was told they needed him to help clean up the existing system first to improve stability before he could work on the new J2EE work. It turned out that the existing system was a 900 page ASP classic application that had been written by people that had grown into a mission critical application. The instability problems turned out to be because none of the shadow IT developers had closed any of the ADO connections in the ASP code, causing the database (I don't know what the database was, but it wouldn't surprise me if it had been MS Access) to crash and burn with too many open connections. Scott's new Job? Go through each and every ASP page and add code by hand to close connections, commands, and recordsets.

I know what you're thinking, just write a fancy Perl script with some regex magic and go on. No such luck, the code was too inconsistent. Just in case you've never been afflicted by classic ASP code, take the worst code you've ever written and put it into a blender. That's about what you'll find in ASP (my ASP code circa 1998 was pretty smelly).


I Don't Think Much of the DAAB

Just to be opinionated, I don't see much value in the original Data Access Application Block or the improved replacement in the Enterprise Library. In my opinion, they just don't add much value. It's still a low level API with not much there to guarantee good data access practices. I know a lot of people like them and they're better than nothing, but there are so many better tools out there. For that matter, I just don't think it is difficult to make a data access layer specific to your application that is superior to the DAAB in terms of connection management, transaction management, error handling, and instrumentation.