Closing an IDataReader after yield return is complete

Question

Actually it doesn't have to be an IDataReader.

I had a function something like this:

public IEnumerable<MyClass> GetObjects() 
{
  IDataReader dr = GetSomeDataReader();
  while (dr.Read())
  {
    yield return new MyClass(dr);
  }
  CloseDBConnections();
}

This was working fine until I refactored it like so:

public IEnumerable<MyClass> GetObjects() 
{
  IDataReader dr = GetSomeDataReader();
  try
  {
    return ProcessReader(dr);
  } finally {
    CloseDBConnections();
  }
}
public IEnumerable<MyClass> ProcessReader(IDataReader dr)
{
  while (dr.Read())
  {
    yield return new MyClass(dr);
  }
}

This doesn't work because when the CloseDBConnections() is executed the enumeration has not yet been processed.

Calling .ToList() on the return from GetObjects is what actually executes the enumeration but by then the connection has already been destroyed and the IDataReader fails.

In my instance CloseDBConnections cannot be called from within the new ProcessReader function because the IDataReader could have come from some other source (the whole point of re-factoring in this instance)

Is there a sensible workaround for this or will I have to duplicate the enumerating code?

I tried putting the call to ProcessReader as yield return instead of return but this doesn't work because C# (understandably) thinks I am trying to add an IEnumerable to the IEnumerable!

score 3 · Answer 1 · answered Jul 25 '13 at 08:44

3

Not pretty, but this should work.

public IEnumerable<MyClass> GetObjects() 
{
  IDataReader dr = GetSomeDataReader();
  try
  {
    foreach (var result in ProcessReader(dr))
    {
      yield return result;
    }
  } finally {
    CloseDBConnections();
  }
}

answered Jul 25 '13 at 08:44

cremor

6,669
1
29
72

yeah I did consider this. I was hoping for a cleaner solution (like some magic keyword or something). In this instance it hardly seems worth the bother of re-factoring if the above solution is the only one possible. I guess I was probably hoping for too much! – DJL Jul 25 '13 at 08:54

sloth · Accepted Answer · 2013-07-25T10:20:43.753

3

How about calling CloseDBConnections in ProcessReader via a callback?

public IEnumerable<MyClass> GetObjects() 
{
  return ProcessReader(GetSomeDataReader(), CloseDBConnections);
}

public IEnumerable<MyClass> ProcessReader(IDataReader dr, Action OnFinished)
{
  try
  {
    while (dr.Read())
      yield return new MyClass(dr);
  }
  finally
  {
    if (OnFinished != null) 
      OnFinished();
  }
}

edited Jul 25 '13 at 10:20

answered Jul 25 '13 at 09:05

sloth

99,095
21
171
219

You should execute the callback in a `finally`. Without that, it won't be executed if the iteration completes early (e.g. `GetObjects().First()`). – svick Jul 25 '13 at 10:19
@DominicKexel I like it. This is much how I end up doing many things in Javascript – DJL Jul 25 '13 at 10:24
@svick I wasn't aware of this - I guess this means the original code should have a same try/finally for the same reason. – DJL Jul 25 '13 at 10:25
@DJL Yeah, you're right, the `finally` is necessary there too. – svick Jul 25 '13 at 10:35

Closing an IDataReader after yield return is complete

2 Answers2