4

I have the following code I use to simulate a live data feed which simultaneously sends a message that each object of type "Symbol" in the collection inside "Portfolio.Symbols" should respond to (by another method doing some work on it).

In order for it to be true simultaneously, I try to register an anonymous event handlers the following way:

static public void RegisterEvents()
{
   foreach (Symbol symbol in Portfolio.Symbols)
   {
      GenerateQuoteRequest += () => { SomeMethod(symbol); };
   }
}

static public void Run()
{
   OnGenerateQuoteRequest();
   Thread.Sleep(100);
}

public delegate void OnGenerateQuoteRequestEventHandler();     
public static event OnGenerateQuoteRequestEventHandler GenerateQuoteRequest 
                                            = delegate {};
...

I then try to raise the event, hoping the I will get a number of "SomeMethod" instances firing up. Unfortunately, only the last "symbol" added is called.

What am I missing here?

Cheng Chen
  • 42,509
  • 16
  • 113
  • 174
Saul
  • 471
  • 1
  • 5
  • 12
  • 2
    only the last "symbol" added is called - Yup, but it is called a lot. – H H Feb 09 '11 at 13:33
  • See also http://stackoverflow.com/questions/3190578/from-eric-lipperts-blog-dont-close-over-the-loop-variable (and the various links in the top and comments). This is a very popular SO question. – Brian Feb 09 '11 at 14:23

1 Answers1

10

The infamous captured-variable/foreach glitch; try:

foreach (Symbol symbol in Portfolio.Symbols)
{
     var copy = symbol;
     GenerateQuoteRequest += () => { SomeMethod(copy); };
}

and btw; static events are really dangerous - those event subscriptions won't unsubscribe themselves, so you could be keeping lots of things in memory unnecessarily. You can make them self-unsubscribing, of course:

foreach (Symbol symbol in Portfolio.Symbols)
{
     var copy = symbol;
     OnGenerateQuoteRequestEventHandler handler = null;
     handler = () => {
         SomeMethod(copy);
         GenerateQuoteRequest -= handler;
     };
     GenerateQuoteRequest += handler;
}
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • @Saul - nothing *really*, except you need to understand that "captures" happen at the declaration scope of the variable, and *technically* the variable `symbol` is declared **outside** the loop; you are capturing 1 **variable** - the same one each time. By moving the scope of `copy` inside the loop, you capture (in terms of the logic) a different variable per iteration. – Marc Gravell Feb 09 '11 at 13:34
  • @Saul - note also my notes on the event - and in all seriousness: static events are rarely a good idea – Marc Gravell Feb 09 '11 at 13:35
  • Weren't they called "closures"? Nice answer though! – Steven Jeuris Feb 09 '11 at 13:39
  • 1
    @Steven Jeuris: A captured-variable in a closure. – Cheng Chen Feb 09 '11 at 13:42