Best practice in declaring events in C#

Question

I know the following two methods work, but I wonder if one is better in terms of performance / maintenance / whatever.

The short way:

  public event EventHandler MyEvent;

The long way:

  private EventHandler _myEvent;
  public event EventHandler MyEvent
  {
     add { _myEvent += value; }
     remove { _myEvent -= value; }
  }

The long way seems similar to encapsulating members with properties, which is indeed a good practice. But does this apply to event handlers?

Answer 1

In this case, the argument of "good practice" is a little tricker; the first is a "field-like event"; you note:

The long way seems similar to encapsulating members with properties,

but: this is encapsulated (behind add/remove) either way; so by comparison to properties, it is really the difference between:

public int Foo {get;set;}

and

private int foo;
public int Foo {
    get { return foo; }
    set { foo = value; }
}

In which case I would say "use the first one unless you have an actual reason not to" - it is still hidden behind accessors. Additionally, it is important to note that your second example is not what a field-like event (the first example) expands to : the compiler adds thread-safety into the mix. So: I would say use the first sample:

public event EventHandler MyEvent;

Note that the "how" for the thread-safety depends on which compiler version (and indeed, which specification) you use. In recent Microsoft C# compilers, it does this with Interlocked operations (CompareExchange etc), so it does not require a dedicated private synchronization object.

Answer 2

The first way does exactly the same underneath with creating private EventHandler with the name MyEvent. When it's accessed within the class event handler is returned (i.e. no problem with calling delegate) when it's called outside of class (myClassInstance.MyEvent += SomeHandler/myClassInstance.MyEvent -= SomeHandler) Add/Remove methods are called respectively. And these methods are the same (except they add thread-safety) as those you've written in the second way.

So why do you want to write more code when you actually don't need it?

Answer 3

To check what Marc Gravel mean i tried the following code:

public event EventHandler MyEventShortWay;

private EventHandler _myEvent;
public event EventHandler MyEventLongWay
{
    add { _myEvent += value; }
    remove { _myEvent -= value; }
}

I was surprised by what was generated (I edited the decompiled variable name) :

private EventHandler _myEventShortWay;

public event EventHandler MyEventShortWay
    {
        add
        {
            EventHandler handler2;
            EventHandler myEventShortWay = this._myEventShortWay;
            do
            {
                handler2 = myEventShortWay;
                EventHandler handler3 = (EventHandler)Delegate.Combine(handler2, value);
                myEventShortWay = Interlocked.CompareExchange<EventHandler>(ref this._myEventShortWay, handler3, handler2);
            }
            while (myEventShortWay != handler2);
        }
        remove
        {
            EventHandler handler2;
            EventHandler myEventShortWay = this._myEventShortWay;
            do
            {
                handler2 = myEventShortWay;
                EventHandler handler3 = (EventHandler)Delegate.Remove(handler2, value);
                myEventShortWay = Interlocked.CompareExchange<EventHandler>(ref this._myEventShortWay, handler3, handler2);
            }
            while (myEventShortWay != handler2);
        }
    }

    private EventHandler _myEvent;

    public event EventHandler MyEventLongWay
    {
        add
        {
            this._myEvent = (EventHandler) Delegate.Combine(this._myEvent, value);
        }
        remove
        {
            this._myEvent = (EventHandler)Delegate.Remove(this._myEvent, value);
        }

    }