views:

118

answers:

6

From what I've read I'm not sure if I've got the naming convention for events and handlers correct. (there seems to be some conflicting advice out there).

In the two classes below can anyone tell me if I've got the naming right for the event, the method that raises the event and the method that handles the event?

public class Car
{
 // is event named correctly?
 public event EventHandler<EventArgs> OnSomethingHasHappened;

 private void MoveForward()
 {
  RaiseSomethingHasHappened();
 }

 // is the named correctly
 private void RaiseSomethingHasHappened()
 {
  if(OnSomethingHasHappened != null)
  {
   OnSomethingHasHappened(this, new EventArgs()); 
  }
 }
}

and the subscriber class:

public class Subscriber()
{
 public Subscriber()
 {
  Car car = new Car();
  car.OnSomethingHasHappened += Car_SomethingHasHappened();
 }

 // is this named correctly?
 private void Car_SomethingHasHappened(object sender, EventArgs e)
 {
  // do stuff
 }
}

Thanks in advance!

A: 

I personally look at how Microsoft has named their events and how they name their handlers.

class Form{
   public event EventHandler<EventArgs> MouseMove;
   public virtual void OnMouseMove()
   {
       if(MouseMove != null)
       {
           MouseMove(this, new EventArgs());
       }
   }
}

class Application{
   public Application()
   {
       Form form = new Form();
       form.MouseMove += //Hook your own Method
   }
}
PieterG
+2  A: 

I tend to do the opposite:

public event EventHandler SomethingHappened;

private void OnSomethingHappened()
{
    SomethingHappened();
}

Then:

private void Car_SomethingHappened()
{

}

Not the cleanest code, but the naming is how I do it. If there isn't a clear local variable name or it doesn't make sense, I suffix the name with "Handler":

private void SomethingHappenedHandler() {}
Adam
+7  A: 

Almost

The method to fire the event - On<When>Event (from RaiseSomethingHasHappened)

i.e. OnBeforeOpen, OnClosing, OnSomethigHasHappened

The event <When>Event (from OnSomethingHasHappened)

i.e. BeforeOpen, Closing, SomethingHAsHappened

the handler <The Instance or meaningful Name><_><Event> (from Car_SomethingHasHappened)

i.e. Form_BeforeOpen, Window_Closing, Car_SomethingHasHappened -> perfect

MaLio
+1 However, in some MS examples the private event firing method is NotifyEventName (e.g. NotifyProperyChanged).
Danny Varod
A: 

I would say the naming convention is okay, but what i miss in you example WHAT happened?

So i would more specialize the name of the event itself (like MovedForward) or if you need it more generalized you should provide some additional information within the EventArgs about what has changed (like the ListChanged in BindingList).

Oliver
+1  A: 

Well, the first point is that you define your own naming convention and there is no 'wrong' way to do it (as long as it's consistent).

Having said that, the Microsoft standards are good if your sharing your code with other.

Normally, you would have events names as:

public class Car
{
 // is event named correctly?
 public event EventHandler<EventArgs> SomethingHasHappened;

 private void MoveForward()
 {
  OnSomethingHasHappened();
 }

 // is the named correctly
 protected virtual void OnSomethingHasHappened()
 {
  EventHandler<EventArgs> locum = SomethingHasHappened;
  if(locum!= null)
  {
   locum(this, new EventArgs()); 
  }
 }
}

Note that the event is titled without the 'On' prefix, and the event firing method is named with the 'On' prefix. The event firing method is also protected virtual so that derived classes can override to change/add to the behaviour as well as use it to fire the event themselves when required.

Dr Herbie
A: 

This is a great article not only about naming events but everything related to events: C# Event Implementation Fundamentals, Best Practices and Conventions

Giorgi