I can’t begin to tell you how many times I hear this from my boss. Really, I should know by now, but I still manage to just keep my head down and code sometimes. Sometimes I get so involved with the code and getting everything working that I don’t keep my eyes open on the big picture.

Currently, we’re working on a system that manages the database itself, so no nHibernate. It uses and Oracle database, so in some data import cases, we were running the exact same queries over and over. Part of the solution was to use data parameters.

Overall, it was quite nice using the parameters. However, I started to notice a really bad trend. In a nutshell it looked something like this:

   1: public IList<IFoo> GetFoosByBarAndBaz(string bar, string baz) {

   2:     IList<IParameter> parameters = parameterFactory.CreateListWithParameterFrom(1, bar, DbType.String);

   3:     parameters.Add(parameterFactory.CreateFrom(2, baz, DbType.String);

   4:

   5:     return MapFoosFrom(gateway.ExecuteSqlForDataTableWithParameters(queries.GetFooByBarAndBaz(), parameters));

   6: }

parameterFactory.CreateListWithParameterFrom(1, bar, DbType.String) as it looks like creates an IList<IParameter> and populates it with a parameter with a name of 1, value of bar and type of String. However, the the Sql string itself sits in an entirely different class. Think about it, how is the next person supposed to know that the parameter :1 is supposed to be bar (yes, we could get into naming here, but there are reasons for using :1, :2 instead of :bar, :baz so bear with me).

The disconnect is huge, and will be nasty for me (or anyone else for that matter) down the line. Plus: I now have the extra work of stubbing out those calls to the parameterFactory in each one of my tests for each of these 50 or so queries.

So I started to think. Yes, I do that sometimes.

We already had an IParameterizedSqlStatement which was used elsewhere.

   1: public interface IParameterizedSqlStatement() {

   2:     string SqlStatement { get; }

   3:     IList<IParameter> Parameters { get; }

   4: }

And on the Gateway:

   1: public interface IGateway {

   2:     DataTable ExecuteSqlForDataTableWithParameters(string sqlStatment, IList<IParameter> parameters);

   3:     DataTable ExecuteSqlForDataTable(IParameterizedSqlStatement parameterizedSqlStatement);

   4: }

And yes, the ExecuteSqlForDataTable(IParameterizedSqlStatement) just delegates to ExecuteSqlForDataTableWIthParameters.

In the end, I took the queries object, and instead of returning just a string of Sql, I build the entire IParameterizedSqlStatement and return it. Now my original function looks like:

   1: public IList<IFoo> GetFoosByBarAndBaz(string bar, string baz) {

   2:     return MapFoosFrom(gateway.ExecuteSqlForDataTable(queries.GetFooByBarAndBaz(bar, baz));

   3: }

I now have 50 functions with 1-3 lines less of code in them, with 1-3 less stubbed calls to make in their tests, plus I was also able to convert the entire application to use IParameterizedSqlStatements, so now the gateway looks like:

   1: public interface IGateway {

   2:     DataTable ExecuteSqlForDataTable(IParameterizedSqlStatement);

   3: }

The other function is still inside my concrete implementation, and the remaining one still delegates to it, but overall my outlying interface is much cleaner, my repository is much cleaner and all the work for creating the query is now inside of the queries object where it belongs. Plus, now my sql string and my generation of the parameters are side by side, so no more tabbing between classes just to make sure that my parameter names are matching in the string and the parameter list.

And the best part is that it didn’t take my boss looking over it and saying “You know, if you just moved this piece over here….”