Disclaimer: None of the SQL refers to data that I use at work. They were inspired by golang's database/sql package documentation

One of the things that I've been learning as a programmer is what well factored code looks like. There are several concerns that go into it, but one very important part of well factored code is the principle of Don't Repeat Yourself (DRY). The net result of DRY is that when you only have to write the unique elements of your code, removing repetitious noise. This helps you focus on actual differences in code, instead of trying to to filter the signal out of the noise.

This sounds fine in theory, but how does it look in practice? My day job involves a lot of SQL queries from C# code. There are several options, but one that I often use to is to use SQL in strings and Stored procedures calls via ADO.NET's SqlDataReader and SqlCommand classes. At a library level, these classes might be considered well factored, since all the variable parts of a SqlCommand are split into different classes:

• SqlConnection, for connecting to databases
• SqlParameter, for protecting from Sql Injection
• SqlDataReader, for gathering the results of a SqlStatement
• SqlCommmand, the glue that holds it all together(attach it to a connection, feed it parameters and execute it to run the command and/or get a SqlDataReader).

The problem with these classes is that if you are using each of them out every time you write a query, you end up with a lot of repetition. For example, below is how wrote my SQL calls in C# until recently (when I discovered the the rest of code in this article).

int age = 27;string eyeColor ="Brown";
SqlConnection conn = new SqlConnection("Data Source=myServerAddress;Initial Catalog=myDataBase;Integrated Security=True;");
SqlCommand getPeople = new SqlCommand("Select name FROM users WHERE age = @Age and eyeColor= @eyeColor");
getPeople.Connection = conn;
SqlParameter ageParam = new SqlParameter("@Age", age);
SqlParameter eyeColorParam = new SqlParameter("@eyeColor", eyeColor)
try
{
    conn.Open();
    getPeople.Parameters.Add(ageParam);
    getPeople.Parameters.Add(eyeColorParam);
    SqlDataReader results = getPeople.ExecuteReader();
    while (results.Read())
    {
        Console.WriteLine("{0} is {1}", (string)results["nane"], age);
    }
}
catch(SqlException ex)
{
    Console.WriteLine("Error: {0}", ex.Message);
    throw;
}
finally
{
    conn.Close();
}

(Yes, I know about using, but when you need to catch errors, finally works just as well)

There are a lot of issues with this code. First of all, if you forget any of the Sql* objects, you'll end with a runtime error. This has (for me at least) resulted in many testing sessions where I run code to find out that "A SqlCommand requires an open connection", or "Command expects parameter @Age, which was not supplied". It's also long, and this means that it will be copy/pasted all over a code base (naturally, nobody wants to type that code twice), and other logic will get mixed into it.

Clearly a helper method is in order. First we want to extract all the pieces that will usually do the same thing. For most applications, this ends up being the connection string. So we can extract that into a helper method. But we should go one step further. When fetching results from a SqlDataReader, you can call ExecuteReader with a CommandBehavior parameter, which allows you to tell the data reader to do different things with the command.

One of the options that it gives you is CommandBehavior.CloseConnection. This tells the SqlDataReader to close the connection when the reader is closed. This allows us to pass a SqlDataReader and not have to worry about the connection (as long as the reader is closed). These two pieces give use the following, which allows us to remove the code that manages the connection string.

public static class Queries{
    SqlConnection conn
    {
        get
        {
            return new SqlConnection("Data Source=myServerAddress;Initial Catalog=myDataBase;Integrated Security=True;");
        }
    }
    //Because the connection is passed in, we don't close it.
    public static SqlDataReader Run(SqlCommand comm)
    {
        comm.Connection = conn;
        //The caller needs to catch exceptions here.
        if (conn.ConnectionState == ConnectionState.Closed)
        {
            conn.Open();
        }
        return comm.ExecuteReader(CommandBehavior.CloseConnection);
    }
}

Also useful when dealing with SqlDataReaders is that you can iterate over them with foreach. This exposes a collection of IDataRecords that point at the reader. The reader is also automatically closed when the foreach loop exits, so we get automatic cleanup. With the CloseConnection behavior we don't have to worry about the connection either. Combine that with our helper method above and we get the following usage:

int age = 27;string eyeColor ="Brown";
SqlCommand getPeople = new SqlCommand("Select name FROM users WHERE age = @Age");
SqlParameter age = new SqlParameter("@Age", age);
SqlParameter eyeColorParam = new SqlParameter("@eyeColor", eyeColor)
getPeople.Parameters.Add(age);
try
{   

    foreach (IDataRecord r in Queries.Run(getPeople))
    {
        Console.WriteLine("{0} is {1}", (string)results["name"], age);
    }
}
catch(SqlException ex)
{
    Console.WriteLine("Error: {0}", ex.Message);
    throw;
}

That clears up several of the Sql- objects that we were dealing with before. We also lose the finally block, as Queries.Run takes care of the connection. But there is potential for more factoring. Why do we need to have the SqlCommand getPeople if we are just going to add a parameter to it and build it from a string?

This suggests a slight modification to the Queries.Run function above. Instead of taking a SqlCommand, it can take a string and an array of SqlParameters, similar to the way String.Format works. I used to params keyword to save the caller from always having to explicitly build an array from the SqlParameters.

public static class Queries{
    SqlConnection newConn()
    {
            return new SqlConnection("Data Source=myServerAddress;Initial Catalog=myDataBase;Integrated Security=True;");
    }
    //The caller is responsible for catching exceptions
    public static SqlDataReader Run(string query, params SqlParameter[] parameters)
    {
        SqlCommand command = new SqlCommand(query);
        command.Connection = newConn();
        if (parameters != null)
        {
            command.Parameters.AddRange(parameters);
        }
        //The caller needs to catch exceptions here.
        return command.ExecuteReader(CommandBehavior.CloseConnection);
    }
}

This gets used like so:

SqlParameter age = new SqlParameter("@Age", 27);SqlParameters eyeColor = new SqlParameter("@eyeColor", "brown")
try
{
    string q = "Select Name from Users where age = @Age"
    foreach(IDatarecord user in Queries.Run(q, age, eyeColor))
    {
        Console.WriteLine("{0} is {1}", (string)user["Name"], age);
    }
}
catch(SqlException ex)
{
    Console.WriteLine("Error: {0}", ex.Message);
    throw;
}

Now we only have 6 lines of code* for the same result as the 16 lines we had before. And we have a method that we can use anywhere we need to pull some results from the database a bit of C# connects to. I think this is a decent factoring of the process of calling SQL code from C#, but it is a bit incomplete compared to ADO.Net. It doesn't handle the need to just run a query without fetching results, or running Stored procedures directly. It could also use a way to run a command with a specific timeout.

It's important to note that adding those to the mix would change how the code should be factored. Since our simple example is just pulling brown-eyed 27 year olds, we don't need that extra stuff. And that's one thing to realize when factoring code: only factor as much as is useful. Beyond that, and you have extraneous yak shaving.

Comments