Remove redundant delegate constructor call?

Question

I downloaded ReSharper and it is telling me to change this line:

dispMap.OnDraw += new EventHandler(dispMap_OnDraw);

To be this line:

dispMap.OnDraw += dispMap_OnDraw;

Because the first line is a "redundant delegate constructor call."

Is this true? In the automatically generated designer code for forms the syntax is based on the first piece of code and when typing in dispMap.OnDraw += and hitting TAB the IDE automatically generates new EventHandler(dispMap_OnDraw)

I'm just curious about this one. Does ReSharper have a point?

Answer 1

Yes, this is correct. I have done this in several cases.

The delegate constructor call should be implicit; the type can be inferred from OnDraw and validated against the method signature of dispMap_OnDraw.

Also, a quote from this MSDN article appears relevant:

Because the += operator merely concatenates the internal invocation list of one delegate to another, you can use the += to add an anonymous method. Note that with anonymous event handling, you cannot remove the event handling method using the -= operator unless the anonymous method was added as a handler by first storing it to a delegate and then registering that delegate with the event.

I believe the delegate instance is created either way, but since you don't have an object reference for the delegate when you implicitly instantiate, you can't remove it with the -= operator.

Answer 2

It does have a point. The second line is shorthand for the first. Depending on your coding standards/conventions, you could use either one, but the first one does add a lot of noise.

Answer 3

If you compare the IL generated in both cases, you'll see that they are the same. Here's both cases in C#, and the IL they result in.

Example C#:

namespace EventTest
{
    public class Publisher
    {
        public delegate void SomeEvent(object sender);
        public event SomeEvent OnSomeEvent;
        public event SomeEvent OnOtherEvent;
    }

    public class Subscriber
    {
        public Subscriber(Publisher p)
        {
            p.OnSomeEvent += new Publisher.SomeEvent(Respond);
            p.OnOtherEvent += Respond;
        }

        public void Respond(object sender)
        {

        }
    }
}

Here's the IL for the constructor. Pay attention to lines IL_000a through IL_0028.

.method public hidebysig specialname rtspecialname 
        instance void  .ctor(class EventTest.Publisher p) cil managed
{
  // Code size       48 (0x30)
  .maxstack  8
  IL_0000:  ldarg.0
  IL_0001:  call       instance void [mscorlib]System.Object::.ctor()
  IL_0006:  nop
  IL_0007:  nop
  IL_0008:  ldarg.1
  IL_0009:  ldarg.0
  IL_000a:  ldftn      instance void EventTest.Subscriber::Respond(object)
  IL_0010:  newobj     instance void EventTest.Publisher/SomeEvent::.ctor(object,
                                                                          native int)
  IL_0015:  callvirt   instance void EventTest.Publisher::add_OnSomeEvent(class EventTest.Publisher/SomeEvent)
  IL_001a:  nop
  IL_001b:  ldarg.1
  IL_001c:  ldarg.0
  IL_001d:  ldftn      instance void EventTest.Subscriber::Respond(object)
  IL_0023:  newobj     instance void EventTest.Publisher/SomeEvent::.ctor(object,
                                                                          native int)
  IL_0028:  callvirt   instance void EventTest.Publisher::add_OnOtherEvent(class EventTest.Publisher/SomeEvent)
  IL_002d:  nop
  IL_002e:  nop
  IL_002f:  ret
} 

Conclusion: I don't see any reason to change your code, they are equivalent.