3

I'm scrutinizing cyclomatic complexity in an application with the purpose of testing anything that's got a cyclomatic complexity greater than 1.

What I'm seeing is that a very simple event's add accessor has a cyclomatic complexity of 2. Why is this? Is it because the add method checks first to see if the callback method is already registered?

I created a very simple calculator application to replicate this behavior. I have an event called CalculateComplete that fires when the Calculate() method is complete.

enter image description here

Trevor
  • 4,620
  • 2
  • 28
  • 37

1 Answers1

1

If there is some class with some event, for example

class SomeClass
{
    public event Action<int> SomeEvent;
}

Then the IL code generated for the event add method is:

SomeClass.add_SomeEvent:
IL_0000:  ldarg.0     
IL_0001:  ldfld       UserQuery+SomeClass.SomeEvent
IL_0006:  stloc.0     
IL_0007:  ldloc.0     
IL_0008:  stloc.1     
IL_0009:  ldloc.1     
IL_000A:  ldarg.1     
IL_000B:  call        System.Delegate.Combine
IL_0010:  castclass   System.Action<System.Int32>
IL_0015:  stloc.2     
IL_0016:  ldarg.0     
IL_0017:  ldflda      UserQuery+SomeClass.SomeEvent
IL_001C:  ldloc.2     
IL_001D:  ldloc.1     
IL_001E:  call        System.Threading.Interlocked.CompareExchange<Action`1>
IL_0023:  stloc.0     
IL_0024:  ldloc.0     
IL_0025:  ldloc.1     
IL_0026:  bne.un.s    IL_0007
IL_0028:  ret

Notice that at the end of the method there is an Interlocked.CompareExchange() call, followed by a "branch if not equal". So yes, there is a branch, therefore a cyclomatic complexity of 2.

You may ask why is it like that? The reason is that delegates are immutable. When you add a method to a delegate, you don't modify the original delegate, but you actually create a combined delegate from the existing one and the provided method, and reassign it to the event. See Delegate.Combine.

And besides that, the swap between the new and old delegates must be thread-safe, that's why the Interlocked.CompareExchange. If the swap failed, try again.

To help, I've translated the IL to C#:

public void add_SomeEvent(Action<int> arg1)
{
    var local0 = this.SomeEvent;
IL_0007:
    var local1 = local0;
    var local2 = (Action<int>)Delegate.Combine(local1, arg1);
    local0 = Interlocked.CompareExchange(ref this.SomeEvent, local2, local1)
    if (local0 != local1) goto IL_0007;
}
vyrp
  • 890
  • 7
  • 15
  • to really translate it into C#, that'd be a `do...while` loop. – Cory Nelson Mar 26 '17 at 07:03
  • Yes, it would be a `do..while` loop. But labels and `goto`s are valid C# as well, and I wanted to leave it as similar to the IL as possible. – vyrp Mar 26 '17 at 07:05