CODEWITHSUNDEEP
c#eventsdelegates
Dmitry Makovetskiyd
Dmitry Makovetskiyd

Reputation: 7053

delegate, event convention that I don't understand

I looked at this example from the C# in nutshell book (http://www.albahari.com/nutshell/ch04.aspx)

using System;

public class PriceChangedEventArgs : EventArgs
{
  public readonly decimal LastPrice;
  public readonly decimal NewPrice;

  public PriceChangedEventArgs (decimal lastPrice, decimal newPrice)
  {
    LastPrice = lastPrice; NewPrice = newPrice;
  }
}

public class Stock
{
  string symbol;
  decimal price;

  public Stock (string symbol) {this.symbol = symbol;}

  public event EventHandler<PriceChangedEventArgs> PriceChanged;

  ****protected virtual void OnPriceChanged (PriceChangedEventArgs e)
  {
    if (PriceChanged != null) PriceChanged (this, e);
  }****

  public decimal Price
  {
    get { return price; }
    set
    {
      if (price == value) return;
      OnPriceChanged (new PriceChangedEventArgs (price, value));
      price = value;
    }  
  }
}

class Test
{
  static void Main()
  {
    Stock stock = new Stock ("THPW");
    stock.Price = 27.10M;
    // register with the PriceChanged event    
    stock.PriceChanged += stock_PriceChanged;
    stock.Price = 31.59M;
  }

  static void stock_PriceChanged (object sender, PriceChangedEventArgs e)
  {
    if ((e.NewPrice - e.LastPrice) / e.LastPrice > 0.1M)
      Console.WriteLine ("Alert, 10% stock price increase!");
  }
}

What I don't understand, is why this convention is used...

  ****protected virtual void OnPriceChanged (PriceChangedEventArgs e)
  {
    if (PriceChanged != null) PriceChanged (this, e);
  }****

Why do I need that method and why do I care to give it the "this" parameter?!? Cant I just attach the event from that class with the method PriceChanged in the test class straight away and skip that method?!?

Upvotes: 4

Views: 221

Answers (3)

Jay
Jay

Reputation: 57919

This is a convenience for invoking the event.

You do need to check that the event has subscribers, and it is typical to pass this as the sender of the event.

Because the same handler can be used for multiple events, passing an instance of the sender is the only way that you could reliable unsubscribe from the event once it has fired.

I think the preferred way to invoke is to assign to a variable first, lest PriceChanged become null after checking, but before invoking:

var handler = PriceChanged;
if(handler != null) handler(this, e);

Upvotes: 4

jgauffin
jgauffin

Reputation: 101150

Null checks are used since a (event) delegate list is not empty but null if there are no subscribers.

However, it's not thread safe. So it can blow up in your face if you start using a BackgroundWorker or any other multi-threaded technique.

I suggest that you use an empty delegate instead:

public event EventHandler<PriceChangedEventArgs> PriceChanged = delegate {};

Since it allows you to just write:

protected virtual void OnPriceChanged (PriceChangedEventArgs e)
{
   PriceChanged (this, e);
}

It's thread safe and the code is more easy to read.

why do I care to give it the "this" parameter?!?

Same event handler might be used by multiple event generators. The sender tells which generate the invocation is for. You should always send the correct event generator as it is expected and you'll break open/closed principle if you don't

Why Do I need that method?

You don't, unless you would duplicate code otherwise (such as generating the EventArgs class)

Upvotes: 3

Reed Copsey
Reed Copsey

Reputation: 564413

You need the null check, since an event will be null until somebody subscribes to it. If you raise it directly and it's null, an exception will be thrown.

This method is used to raise the event, not to subscribe to it. You can subscribe to the event from another class easily:

yourObject.PriceChanged += someMethodWithTheAppropriateSignature;

However, when you want to have the event "fire", the class needs to raise the event. The "this" parameter is providing the sender argument in the EventHandler<T>. By convention, delegates used for events have two parameters, the first is object sender, which should be the object that raised the event. The second is EventArgs or a subclass of EventArgs, which provides the information specific to that event. The method is used to properly check for null and raise the event with the appropriate information.

In this case, your event is declared as:

public event EventHandler<PriceChangedEventArgs> PriceChanged;

EventHandler<PriceChangedEventArgs> is a delegate which has a signature of:

public delegate void EventHandler<T>(object sender, T args) where T : EventArgs

This means that the event must be raised with two parameters - an object (the sender, or "this"), and an instance of PriceChangedEventArgs.

That being said, this convention is not actually the "best" way to raise the event. It would actually be better to use:

protected virtual void OnPriceChanged (PriceChangedEventArgs e)
{
    var eventHandler = this.PriceChanged;
    if (eventHandler != null) 
        eventHandler(this, e);
}

This protects you in multi-threaded scenarios, since it's possible that a single subscribe could actually unsubscribe in between your null check and the raise if you have multiple threads operating.

Upvotes: 6

Related Questions