ADO.NET best practices – Reading data from data reader

I have seen many people using DataReader incorrectly. In this post, I will try to explain some good practices that can be followed when reading from a data reader. Consider the following problematic code,

SqlDataReader reader = /* ... */;
while (reader.Read())
{
    string userName = reader["user_name"].ToString();
    int age = int.Parse( reader["age"].ToString() );
    /* ... */
}
reader.Close();

How many problems can you figure out from the above code? There are many problems with this code,

  1. The columns “user_name” and “age” may or may not exist. If the column does not exist in the reader, it will throw error.
  2. You may be calling ToString() on an object which may be pointing to NULL. This will lead to a null reference exception.
  3. SqlDataReader implements IDisposable and user code has to call Dispose() each time to release the resources deterministically. This is not happening here.
  4. reader["age"] may contain values that are not compatible with integer. In such case, int.Parse() will throw a FormatException.

Following sections will show a safe method to read data from DataReader.

Understanding ordinals

Ordinal is the index of a column in the reader. GetOrdinal() method will give you the ordinal for the column name supplied. So the first step in reading data from reader should be to find the ordinal of the columns which we want to read. Before getting the ordinal values, you have to ensure that the reader can read. SqlDataReader provides a HasRows property which can be useful here. Here is how you read the ordinals

SqlDataReader reader = /* ... */;
int userNameOrdinal;
int ageOrdinal;
if (reader.HasRows)
{
     try
     {
           userNameOrdinal = reader.GetOrdinal("user_name");
     }
     catch (IndexOutOfRangeException)
     {
           throw new YourCustomException("Expected column 'user_name' not found");
     }
     try
     {
           ageOrdinal = reader.GetOrdinal("age");
     }
     catch (IndexOutOfRangeException)
     {
           throw new YourCustomException("Expected column 'age' not found");
     }
}

If you have support to “Extension methods”, this code can be simplified further.

public static class SqlDataReaderExtensions
{
    public static int GetOrdinalOrThrow(this SqlDataReader reader, string columnName)
    {
        try
        {
            return reader.GetOrdinal(columnName);
        }
        catch (IndexOutOfRangeException)
        {
            throw new YourCustomException(string.Format("Expected column '{0}' not found",columnName));
        }
    }
}

SqlDataReader reader = /* ... */;
int userNameOrdinal;
int ageOrdinal;
if (reader.HasRows)
{
      userNameOrdinal = reader.GetOrdinalOrThrow("user_name");
      ageOrdinal = reader.GetOrdinalOrThrow("age");
}

This makes the code more clean. However, I really wish to see a TryGetOrdinal() method on IDataReader so that we can save the cost of exception handling.

GetOrdinal() method first does a case-sensitive lookup for the column name. If it fails, case-insensitive search is performed. Thus, using the column name in correct case with GetOrdinal() will be more efficient.

Respecting type safety

C# is a strongly typed language and each programmer should take full advantage of this. Reading data like, reader["user_name"] is not a type safe way as it returns a object. Data reader provides methods to read data in a type safe way.

Here is what MSDN says about it

When accessing column data use the typed accessors like GetString, GetInt32, and so on. This saves you the processing required to cast the Object returned from GetValue as a particular type.

Now let us follow the above suggestion and rewrite our code like,

while (reader.Read())
{
   string userName = reader.GetString(userNameOrdinal);
   int age = -1;
   try
   {
         age = reader.GetInt32(ageOrdinal);
   }
   catch (InvalidCastException)
   {
         throw new YourCustomException("Unable to read age. Expecting integer value");
   }
   /* ... */
}

Releasing resources

If any type implements IDisposable, it is like saying “I have something to release explicitly rather than garbage collector to release it“. So it is programmers duty to ensure the calls to Dispose() when using disposable types. C# has “using” statement (stack semantics can be used in C++/CLI) which will ensure calls to Dispose(). Since DataReader is a disposable object, we can write like

using(SqlDataReader reader = command.ExecuteReader())
{
      while (reader.Read())
      {
          string userName = reader.GetString(userNameOrdinal);
          int age = -1;
          try
          {
              age = reader.GetInt32(ageOrdinal);
          }
          catch (InvalidCastException)
          {
              throw new YourCustomException("Unable to read age. Expecting integer value");
          }
          /* ... */
      }
}

This ensures that the reader is disposed properly even in exceptional situations.

Putting everything together

If you put everything together, code will be like

public static class SqlDataReaderExtensions
{
    public static int GetOrdinalOrThrow(this SqlDataReader reader, string columnName)
    {
        try
        {
            return reader.GetOrdinal(columnName);
        }
        catch (IndexOutOfRangeException)
        {
            throw new YourCustomException(string.Format("Expected column '{0}' not found",columnName));
        }
    }
}

int userNameOrdinal = -1;
int ageOrdinal = -1;
if (reader.HasRows)
{
    userNameOrdinal = reader.GetOrdinalOrThrow("user_name");
    ageOrdinal = reader.GetOrdinalOrThrow("age");
}
using (SqlDataReader reader = GetReader())
{
    while (reader.Read())
    {
        string userName = reader.GetString(userNameOrdinal);
        int age = -1;
        try
        {
            age = reader.GetInt32(ageOrdinal);
        }
        catch (InvalidCastException)
        {
            throw new YourCustomException("Unable to read age. Expecting integer value");
        }
        /* ... */
    }
}

We have got clean code now. It handles the exceptions and throws informative exceptions to the caller rather than throwing Boneheaded exceptions, we have avoided explicit casting and DataReader is disposed properly.

These are applicable for all types which implements IDataReader. SqlDataReader is used in this post just for explanation.

Any criticisms are most welcome.

Happy programming!

Advertisements

11 thoughts on “ADO.NET best practices – Reading data from data reader

    • I am not sure why you can’t read this page? Broken formatting? I don’t see any issues here at my end. Please let me know.

  1. > Again, that’s right. Your username can’t possibly be null, so it’s correct for the program just to fail in that case.

    Yeah. But providing a meaningful error message rather than horrible NullReferenceException is always good. How easy is that to find out what went wrong from a NullReferenceException?

    > It’s frankly difficult to understand why you wouldn’t store your user age as an integer. Again, it’s correct fpr the program to fail if you don’t.

    That’s correct too. But here database is an external system. So you ensure your applications expectations are correct by making sure the value has the expected type rather than relying on external system to send the correct data.

    > In conclusion, you’ve transformed an 8 lines, very readable and reasonably safe code (apart from the disposal of the reader), into an ugly 40 lines code without providing any real advantage. To me it’s a fail.

    Well, writing correct code always ends up in more lines. My point was writing correct code rather than some code which works with lots of assumptions about the data it gets and always assumes they send correct data.

    • “But providing a meaningful error message rather than horrible NullReferenceException is always good. How easy is that to find out what went wrong from a NullReferenceException?”

      Hmm, in case you’re trying to access a non existent column, you’ll get an IndexOutOfRangeException with the clear indication of the non existing column you tried to access.
      In the case of a NullReferenceException you’ll get the stack trace so I don’t think there’s anything to be worried about.

      More in general, an exception can be ugly to see, but unless it’s something you expect to go wrong from time to time such as, for example, a network timeout, there’s no need to convert a message a developer can understand immediately to something more user friendly. You’re trading the readability and maintainability of your code for the user friendliness of an exception that may never happen and that no end user should see anyway.

      As for not trusting the external system, that’s a different matter. In most applications you don’t have any reason to be suspicious about the data of the external system (very often you’ve built it yourself). In my view, there must be a very specific reason to waste your time and disrupt the readability of your code to ensure that some trivial expectations about the various parts of the application are met.

      “Well, writing correct code always ends up in more lines.”
      Hmmmm. In my career, so far, I’ve always seen the more verbose code associated with the lowest “correctness” factors, for various reasons, one probably being the fact that cluttered code is harder to understand and to maintain.

  2. Man, you are great. From now on I will use your datareader schema on my project. Thank for your hard work.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s