Performance Optimization – Checking Item Count Before Foreach Loop

cperformancerefactoring

I saw this in code and was wondering if there is any performance benefit to checking the item count prior to looping:

if (SqlParams.Count > 0)
    foreach (var prm in SqlParams)
        cmd.Parameters.Add(prm);

I always prefer to do a null check instead and let the foreach loop just hop out if there are 0 items.

if (SqlParams != null)
    foreach (var prm in SqlParams)
        cmd.Parameters.Add(prm);

Isn't that the better way?

Best Answer

Ultimately, the best answer is to actually test it. Make a method which loops over an empty array with and without checking the length first, call each 100,000 times and see which has a faster runtime.

public void withCheck(Integer[] array) {
    for (int i = 0; i < 100000; i++) {
        if (array.length > 0) {
            for (Integer i : array) {
                // nothing to do.
            }
        }
    }
}

public void withoutCheck(Integer[] array) {
    for (int i = 0; i < 100000; i++) {
        for (Integer i : array) {
            // nothing to do.
        }
    }
}

public void test() {
    long startTime = System.currentTimeMillis();
    withCheck();
    System.out.println("Time with check: " + (System.currentTimeMillis() - startTime) + "ms");
    startTime = System.currentTimeMillis();
    withoutCheck();
    System.out.println("Time without check: " + (System.currentTimeMillis() - startTime) + "ms");
}

When I write code like this, I generally ponder this as a trade-off between code readability and perceived or potential performance. In other words, adding a if (count > 0) check negatively impacts the readability, even though by a very small amount. So what is the gain? I presume there is very little or no advantage, but like you, I have not actually tested it. What I can tell you is that in all my years profiling, I have never found looping over an empty array to be a bottleneck.

Technically, it depends on how the iterator works. For a foreach loop like you have, you are creating an iterator object behind the scenes. My expectation would be, and again, this is unverified, is that the iterator object FOR AN ARRAY would simply be implemented like this (this is Java, but C# is probably comparable):

public class ArrayIterator<T> implements Iterator<T> {
    private int next = 0;
    public T next() {
        return backingArray[next++];
    }

    public boolean hasNext() {
        return next < backingArray.length;
    }
}

So iterating over such an iterator should exit out very quickly because hasNext() will return false. Notice it's the same check that your if statement would do.

Now if you are iterating over a linked list, the implementation should be different. If you explicitly check the size, you may be forcing it to scan the linked list. In Java, LinkedList.size() checks an explicit size variable so it doesn't walk the list, but I don't know how .NET implements it.

Another way to look at this is how likely is the collection to be empty? If that is a rare case, then if the theory that iterating over an empty array takes time is true, then adding an explicit check for empty first would only be faster if the collection is usually empty. If not, you are theoretically slowing down your average case just to slightly speed up your worst case.

Again, these are all theories and the timings are on the micro-level, so take it with a grain of salt.

TL;DR

It depends on the type of collection, the runtime platform, and the typical collection size, but intuitively, it seems unlikely to speed anything up in most situations. But the only way to know for sure is to test it.

Related Topic