Rory Primrose

Learn from my mistakes, you don't have time to make them yourself

View project on GitHub

Code coverage doesn't like foreach loops

I have an interesting scenario that I have just come across in my code. I have a foreach loop that is not getting 100% code coverage in unit tests. Prior to this, I really liked foreach for its ease of use and readability even though there is a minor performance penalty compared to using a for loop.

Here is the situation. I have a flush method that looks like this:

public void Flush()
{
    // Loop through each listener
    foreach (TraceListener listener in Source.Listeners)
    {
        // Flush the listener
        listener.Flush();
    }
}

Code coverage for this method says that 2 blocks not covered, 12.5% not covered, 14 blocks covered, 87.5% covered. Code metrics for this method are maintainability index is 80, cyclomatic complexity is 3, class coupling is 5 and lines of code is 2.

The IL for this method is:

.method public hidebysig instance void Flush() cil managed
{
    .maxstack 2
    .locals init (
        [0] class [System]System.Diagnostics.TraceListener listener,
        [1] class [mscorlib]System.Collections.IEnumerator CS$5$0000,
        [2] bool CS$4$0001,
        [3] class [mscorlib]System.IDisposable CS$0$0002)
    L_0000: nop 
    L_0001: nop
    L_0002: ldarg.0
    L_0003: call instance class [System]System.Diagnostics.TraceSource MyNamespace.MyClass::get_Source()
    L_0008: callvirt instance class [System]System.Diagnostics.TraceListenerCollection [System]System.Diagnostics.TraceSource::get_Listeners()
    L_000d: callvirt instance class [mscorlib]System.Collections.IEnumerator [System]System.Diagnostics.TraceListenerCollection::GetEnumerator()
    L_0012: stloc.1 
    L_0013: br.s L_002a
    L_0015: ldloc.1 
    L_0016: callvirt instance object [mscorlib]System.Collections.IEnumerator::get_Current()
    L_001b: castclass [System]System.Diagnostics.TraceListener
    L_0020: stloc.0 
    L_0021: nop 
    L_0022: ldloc.0 
    L_0023: callvirt instance void [System]System.Diagnostics.TraceListener::Flush()
    L_0028: nop 
    L_0029: nop 
    L_002a: ldloc.1 
    L_002b: callvirt instance bool [mscorlib]System.Collections.IEnumerator::MoveNext()
    L_0030: stloc.2 
    L_0031: ldloc.2 
    L_0032: brtrue.s L_0015
    L_0034: leave.s L_004d
    L_0036: ldloc.1 
    L_0037: isinst [mscorlib]System.IDisposable
    L_003c: stloc.3 
    L_003d: ldloc.3 
    L_003e: ldnull 
    L_003f: ceq 
    L_0041: stloc.2 
    L_0042: ldloc.2 
    L_0043: brtrue.s L_004c
    L_0045: ldloc.3 
    L_0046: callvirt instance void [mscorlib]System.IDisposable::Dispose()
    L_004b: nop 
    L_004c: endfinally 
    L_004d: nop 
    L_004e: ret 
    .try L_0013 to L_0036 finally handler L_0036 to L_004d
}

The UI for code coverage indicates that each line of code is hit. My guess is that there is something to do with the IEnumerator that is called when foreach is compiled.

I changed the code to this:

public void Flush()
{
    // Loop through each listener
    for (Int32 index = 0; index < Source.Listeners.Count; index++ )
    {
        TraceListener listener = Source.Listeners[index];
    
        // Flush the listener
        listener.Flush();
    }
}

Code coverage now says that 0 blocks not covered, 0% not covered, 11 blocks covered, 100% covered. Code metrics for this method now say that maintainability index is 75, cyclomatic complexity is2, class coupling is 3 and lines of code is 3.

The IL for this method is now:

.method public hidebysig instance void Flush() cil managed
{
    .maxstack 2
    .locals init (
        [0] int32 index,
        [1] class [System]System.Diagnostics.TraceListener listener,
        [2] bool CS$4$0000)
    L_0000: nop 
    L_0001: ldc.i4.0 
    L_0002: stloc.0 
    L_0003: br.s L_0024
    L_0005: nop 
    L_0006: ldarg.0 
    L_0007: call instance class [System]System.Diagnostics.TraceSource MyNamespace.MyClass::get_Source()
    L_000c: callvirt instance class [System]System.Diagnostics.TraceListenerCollection [System]System.Diagnostics.TraceSource::get_Listeners()
    L_0011: ldloc.0 
    L_0012: callvirt instance class [System]System.Diagnostics.TraceListener [System]System.Diagnostics.TraceListenerCollection::get_Item(int32)
    L_0017: stloc.1 
    L_0018: ldloc.1 
    L_0019: callvirt instance void [System]System.Diagnostics.TraceListener::Flush()
    L_001e: nop 
    L_001f: nop 
    L_0020: ldloc.0 
    L_0021: ldc.i4.1 
    L_0022: add 
    L_0023: stloc.0 
    L_0024: ldloc.0 
    L_0025: ldarg.0 
    L_0026: call instance class [System]System.Diagnostics.TraceSource MyNamespace.MyClass::get_Source()
    L_002b: callvirt instance class [System]System.Diagnostics.TraceListenerCollection [System]System.Diagnostics.TraceSource::get_Listeners()
    L_0030: callvirt instance int32 [System]System.Diagnostics.TraceListenerCollection::get_Count()
    L_0035: clt 
    L_0037: stloc.2 
    L_0038: ldloc.2 
    L_0039: brtrue.s L_0005
    L_003b: ret
}

There are several posts around that talk about the performance difference of foreach vs for, but no-one seems to have actually posted metrics to base their stance on. One post that was in interesting read was How to Write High-Performance C# Code by Jeff Varszegi. As far as performance goes, the collection in this situation is always going to be very small so it is perhaps not that much of an issue.

I think that for loops would be faster after looking at the IL and understanding what foreach does under the covers. I don’t think however that the performance difference is significant in itself. However, if foreach causes issues with code coverage, perhaps both these issues combined is enough of a reason to change coding practices.

Updated: Reformatted the IL code to avoid PRE tags that don’t wrap.

Written on April 4, 2008