Yield return and method state

Question

I was trying to remove some duplication from my database execution methods. I had a bunch of methods with the following structure:

IDbConnection connection = mConnections[pConnectionID];
bool wasAlreadyOpen = connection.State == ConnectionState.Open;

try
{
   if (!wasAlreadyOpen)
      connection.Open();

   using (IDbCommand command = connection.CreateCommand())
   {
      command.CommandText = pSQL;
      if (pParams != null)
         ApplyParameters(pParams, command);
      // do something interesting with command
   }
}
finally
{
   if (!wasAlreadyOpen)
      connection.Close();
}

I extracted this logic into another method with this signature:

private object ExecuteQuery(int pConnectionID, string pSQL,
   Func<IDbCommand, object> pQuery, IEnumerable<QueryParameter> pParams)

and in the // do something part of the algorithm, does this:

return pQuery(command);

This seems to work great, except for one issue. In my ExecuteReader method, the query code looks like this:

using (IDataReader reader = command.ExecuteReader())
   if (reader != null)
      while (reader.Read())
         yield return reader;

The problem seems to be that the "state" that is saved for yield return to execute lazily is only taken from the method which contains the yield statement. If I extract those four lines above to its own method, or anonymous method/lambda, then there isn't enough state to keep the database connection open while data is being read.

Is there a way to extract this logic the way I am doing this, or am I only left with inlining this particular method and ignoring the duplication?

Answer 1

My solution to this is to not lazily load from the database. I have a feeling it's not a good idea to load from a database lazily anyway. Instead I have added to the ExecuteReader method a transform function Func<IDataRecord, T> parameter. The data records that are read are then immediately converted into objects, rather than expecting the caller to take an IEnumerable<IDataRecord> and do something with it.

I liked the concision of the yield return version, but I think ultimately it is better not to lazily load from the DB.

Answer 2

What is the point in returning the same copy of reader again and again ?

WRONG Logic

using (IDataReader reader = command.ExecuteReader())
   if (reader != null)
      while (reader.Read())
         yield return reader;

What you are doing is returning the same DataReader object multiple times.

what you need to do is create Class Object from reader object and return the read data.

pseudo-code

using (IDataReader reader = command.ExecuteReader())
   if (reader != null)
      while (reader.Read())
      {
           MyObject obj = new MyObject(reader.getInt32(0), reader.getString(1), reader.getFloat(2));
           yield return obj;
      }

Answer 3

First, I think you should make your method generic. That is, your method shouldn't return object, it should return T:

private T ExecuteQuery<T>(int pConnectionID, string pSQL,
   Func<IDbCommand, T> pQuery, IEnumerable<QueryParameter> pParams)

Now, what I think you should do is to add another overload of ExecuteQuery specifically for collections:

private IEnumerable<T> ExecuteQuery<T>(int pConnectionID, string pSQL,
   Func<IDbCommand, IEnumerable<T>> pQuery, IEnumerable<QueryParameter> pParams)

which would be implemented using yield return itself. Something like:

using (IDbCommand command = connection.CreateCommand())
{
   command.CommandText = pSQL;
   if (pParams != null)
      ApplyParameters(pParams, command);

    foreach (var x in pQuery(command))
        yield return x;
}

This way, the command will be disposed only after iteration of the result completes (or if it's aborted prematurely).

(I'm not completely sure about this, but there is a chance that overload resolution will pick the wrong overload for collections. In that case, rename the collection version to something like ExecuteQueryCollection.)