Events - naming convention and style

Question

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

Answer 1

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:

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.

Answer 2

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);