Events - naming convention and style

I'm learning about Events / Delegates in C#. Could I ask your opinion on the naming/coding style I've chosen (taken from the Head First C# book)?

Am teaching a friend about this tomorrow, and am trying to come up with the most elegant way of explaining the concepts. (thought the best way to understand a subject is to try and teach it!)

class Program
    {
        static void Main()
        {
            // setup the metronome and make sure the EventHandler delegate is ready
            Metronome metronome = new Metronome();

            // wires up the metronome_Tick method to the EventHandler delegate
            Listener listener = new Listener(metronome);
            metronome.OnTick();
        }
    }

---

public class Metronome
    {
        // a delegate
        // so every time Tick is called, the runtime calls another method
        // in this case Listener.metronome_Tick
        public event EventHandler Tick;

        public void OnTick()
        {
            while (true)
            {
                Thread.Sleep(2000);
                // because using EventHandler delegate, need to include the sending object and eventargs 
                // although we are not using them
                Tick(this, EventArgs.Empty);
            }
        }
    }

---

public class Listener
    {
        public Listener(Metronome metronome)
        {
            metronome.Tick += new EventHandler(metronome_Tick);
        }
        
        private void metronome_Tick(object sender, EventArgs e)
        {
            Console.WriteLine("Heard it");
        }
    }

n.b. Code is refactored from http://www.codeproject.com/KB/cs/simplesteventexample.aspx

Microsoft has actually written extensive set of naming guidelines and put it in the MSDN library. You can find the articles here: Naming Guidelines

Aside from the general capitalization guidelines, here is what it has for 'Events' on the page Names of Type Members:

> ✔️ DO name events with a verb or a verb phrase. > > Examples include Clicked, Painting, DroppedDown, and so on. > > ✔️ DO give events names with a concept of before and after, using the present and past tenses. > > For example, a close event that is raised before a window is closed would be called Closing, and one that is raised after the window is closed would be called Closed. > > ❌ DO NOT use "Before" or "After" prefixes or postfixes to indicate pre- and post-events. Use present and past tenses as just described. > > ✔️ DO name event handlers (delegates used as types of events) with the "EventHandler" suffix, as shown in the following example: > > lang-cs > public delegate void ClickedEventHandler(object sender, ClickedEventArgs e); > > > ✔️ DO use two parameters named sender and e in event handlers. > > The sender parameter represents the object that raised the event. The sender parameter is typically of type object, even if it is possible to employ a more specific type. > > ✔️ DO name event argument classes with the "EventArgs" suffix.

There are a few points that I would mention:

Metronome.OnTick doesn't seem to be named correctly. Semantically, "OnTick" tells me it will be called when it "Tick"s, but that isn't really what's happening. I would call it "Go" instead.

The typically accepted model, however would be to do the following. OnTick is a virtual method that raises the event. This way, you can override the default behavior in inherited classes easily, and call the base to raise the event.

class Metronome
{
    public event EventHandler Tick;

    protected virtual void OnTick(EventArgs e)
    {
        //Raise the Tick event (see below for an explanation of this)
        var tickEvent = Tick;
        if(tickEvent != null)
            tickEvent(this, e);
    }

    public void Go()
    {
        while(true)
        {
            Thread.Sleep(2000);
            OnTick(EventArgs.Empty); //Raises the Tick event
        }
    }
}

---

Also, I know this is a simple example, but if there are no listeners attached, your code will throw on Tick(this, EventArgs.Empty). You should at least include a null guard to check for listeners:

if(Tick != null)
    Tick(this, EventArgs.Empty);

However, this is still vulnerable in a multithreaded environment if the listener is unregistered between the guard and the invocation. The best would be to capture the current listeners first and call them:

var tickEvent = Tick;
if(tickEvent != null)
    tickEvent(this, EventArgs.Empty);

I know this is an old answer, but since it's still gathering upvotes, here's the C# 6 way of doing things. The whole "guard" concept can be replaced with a conditional method call and the compiler does indeed do the Right Thing(TM) in regards to capturing the listeners:

Tick?.Invoke(this, EventArgs.Empty);

I would say the best guide to events in general, including naming conventions, is here.

It is the convention I have adopted, briefly:

• Events names are typically terminated with a verb ending with -ing or -ed (Closing/Closed, Loading/Loaded)
• The class which declares the event should have a protected virtual On[EventName] which should be used by the rest of the class for raising the event. This method can be also used by subclasses to raise the event, and also overloaded to modify the event-raising logic.
• There is often confusion about the use of 'Handler' - for coherence, all delegates should be postfixed with Handler, try to avoid calling the methods which implement the Handler 'handlers'
• The default VS naming convention for the method which implements the handler is EventPublisherName_EventName.

Interesting how Microsoft seems to break its own naming conventions with Visual Studio generated event handler names.

See: Event Naming Guidelines (.NET Framework 1.1)

A point I have found after using events in .Net for many years is the repetitive need to check the event for a null handler on every invocation. I'm yet to see a piece of live code that does anything but not call the event if it is null.

What I have started doing is to put a dummy handler on every event I create to save the need to do the null check.

public class Metronome
{
    public event EventHandler Tick += (s,e) => {};

    protected virtual void OnTick(EventArgs e)
    {
        Tick(this, e);  // now it's safe to call without the null check.
    }
}

Looks good, aside from the fact that OnTick doesn't follow the typical event invocation model. Typically, On[EventName] raises the event a single time, like

protected virtual void OnTick(EventArgs e)
{
    if(Tick != null) Tick(this, e);
}

Consider creating this method, and renaming your existing "OnTick" method to "StartTick", and instead of invoking Tick directly from StartTick, call OnTick(EventArgs.Empty) from the StartTick method.

In your case it could be:

class Metronome {
  event Action Ticked;
  
  internalMethod() {
    // bla bla
    Ticked();
  }
}

---

Above sampple use below convention, self-describing ;]

Events source:

class Door {

  // case1: property change, pattern: xxxChanged
  public event Action<bool> LockStateChanged;

  // case2: pure action, pattern: "past verb"
  public event Action<bool> Opened;
  
  internalMethodGeneratingEvents() {
    // bla bla ...
    
    Opened(true);
    LockStateChanged(false);
  }
  
}

BTW. keyword event is optional but enables distinguishing 'events' from 'callbacks'

Events listener:

class AlarmManager {

  // pattern: NotifyXxx
  public NotifyLockStateChanged(bool state) {
    // ...
  }

  // pattern: [as above]      
  public NotifyOpened(bool opened) {
  // OR
  public NotifyDoorOpened(bool opened) {
    // ...
  }

}

And binding [code looks human friendly]

door.LockStateChanged += alarmManager.NotifyLockStateChanged;
door.Moved += alarmManager.NotifyDoorOpened;

Even manually sending events is "human readable".

alarmManager.NotifyDoorOpened(true);

Sometimes more expressive can be "verb + ing"

dataGenerator.DataWaiting += dataGenerator.NotifyDataWaiting;

Whichever convention you choose, be consistent with it.