How to properly check IEnumerable for existing results

Question

What's the best practice to check if a collection has items?

Here's an example of what I have:

var terminalsToSync = TerminalAction.GetAllTerminals();

if(terminalsToSync.Any())
    SyncTerminals(terminalsToSync);
else
    GatewayLogAction.WriteLogInfo(Messages.NoTerminalsForSync);

The GetAllTerminals() method will execute a stored procedure and, if we return a result, (Any() is true), SyncTerminals() will loop through the elements; thus enumerating it again and executing the stored procedure for the second time.

What's the best way to avoid this?

I'd like a good solution that can be used in other cases too; possibly without converting it to List.

Thanks in advance.

Answer 1

I would probably use a ToArray call, and then check Length; you're going to enumerate all the results anyway so why not do it early? However, since you've said you want to avoid early realisation of the enumerable...

I'm guessing that SyncTerminals has a foreach, in which case you can write it something like this:

bool any = false;
foreach(var terminal in terminalsToSync)
{
  if(!any)any = true;
  //....
}

if(!any)
  GatewayLogAction.WriteLogInfo(Messages.NoTerminalsForSync);

Okay, there's a redundant if after the first loop, but I'm guessing the cost of an extra few CPU cycles isn't going to matter much.

Equally, you could do the iteration the old way and use a do...while loop and GetEnumerator; taking the first iteration out of the loop; that way there are literally no wasted operations:

var enumerator = terminalsToSync.GetEnumerator();
if(enumerator.MoveNext())
{
  do
  {
    //sync enumerator.Current
  } while(enumerator.MoveNext())
}
else
  GatewayLogAction.WriteLogInfo(Messages.NoTerminalsForSync);

Answer 2

Personally i wouldnt use an any here, foreach will simply not loop through any items if the collection is empty, so i would just do it like that. However i would recommend that you check for null.

If you do want to pre-enumerate the set use .ToArray() eg will only enumerate once:

var terminalsToSync = TerminalAction.GetAllTerminals().ToArray();

if(terminalsToSync.Any())
    SyncTerminals(terminalsToSync);

Answer 3

How about this, which still defers execution, but buffers it once executed:

var terminalsToSync = TerminalAction.GetAllTerminals().Lazily();

with:

public static class LazyEnumerable {
    public static IEnumerable<T> Lazily<T>(this IEnumerable<T> source) {
        if (source is LazyWrapper<T>) return source;
        return new LazyWrapper<T>(source);
    }
    class LazyWrapper<T> : IEnumerable<T> {
        private IEnumerable<T> source;
        private bool executed;
        public LazyWrapper(IEnumerable<T> source) {
            if (source == null) throw new ArgumentNullException("source");
            this.source = source;
        }
        IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); }
        public IEnumerator<T> GetEnumerator() {
            if (!executed) {
                executed = true;
                source = source.ToList();
            }
            return source.GetEnumerator();
        }
    }
}

Answer 4

var terminalsToSync = TerminalAction.GetAllTerminals().ToList();

if(terminalsToSync.Any())
    SyncTerminals(terminalsToSync);
else
    GatewayLogAction.WriteLogInfo(Messages.NoTerminalsForSync);

Answer 5

All the caching solutions here are caching all items when the first item is being retrieved. It it really lazy if you cache each single item while the items of the list are is iterated.

The difference can be seen in this example:

public class LazyListTest
{
    private int _count = 0;

    public void Test()
    {
        var numbers = Enumerable.Range(1, 40);
        var numbersQuery = numbers.Select(GetElement).ToLazyList(); // Cache lazy
        var total = numbersQuery.Take(3)
            .Concat(numbersQuery.Take(10))
            .Concat(numbersQuery.Take(3))
            .Sum();
        Console.WriteLine(_count);
    }

    private int GetElement(int value)
    {
        _count++;
        // Some slow stuff here...
        return value * 100;
    }
}

If you run the Test() method, the _count is only 10. Without caching it would be 16 and with .ToList() it would be 40!

An example of the implementation of LazyList can be found here.

Answer 6

.Length or .Count is faster since it doesn't need to go through the GetEnumerator()/MoveNext()/Dispose() required by Any()

Answer 7

Here's another way of approaching this problem:

int count = SyncTerminals(terminalsToSync);
if(count == 0)  GatewayLogAction.WriteLogInfo(Messages.NoTerminalsForSync);

where you change SyncTerminals to do:

int count = 0;
foreach(var obj in terminalsToSync) {
    count++;
    // some code
}
return count;

Nice and simple.

Answer 8

If you're seeing two procedure calls for the evaluation of whatever GetAllTerminals() returns, this means that the procedure's result isn't being cached. Without knowing what data-access strategy you're using, this is quite hard to fix in a general way.

The simplest solution, as you've alluded, is to copy the result of the call before you perform any other operations. If you wanted to, you could neatly wrap this behaviour up in an IEnumerable<T> which executes the inner enumerable call just once:

public class CachedEnumerable<T> : IEnumerable<T>
{
    public CachedEnumerable<T>(IEnumerable<T> enumerable)
    {
        result = new Lazy<List<T>>(() => enumerable.ToList());
    }

    private Lazy<List<T>> result;

    public IEnumerator<T> GetEnumerator()
    {
        return this.result.Value.GetEnumerator();
    }

    System.Collections.IEnumerable GetEnumerator()
    {
        return this.GetEnumerator();
    }
}

Wrap the result in an instance of this type and it will not evaluate the inner enumerable multiple times.