0

Consider the following code:

public ViewModel()
{
    PropertyChanged += new PropertyChangedEventHandler(ViewModel_PropertyChanged);
    PropertyChanged += ViewModel_PropertyChanged;
}

void ViewModel_PropertyChanged(object sender, PropertyChangedEventArgs e)
{
    throw new NotImplementedException();
}

In VisualStudio, if you type PropertyChanged +=, the application prompts you to press tab to generate an event handler that looks like the first line of my constructor. However, it is also valid (and more concise) to write this like the second line in my constructor.

Do these two lines have different meaning? If not, why does VisualStudio like to generate the former?

Oliver
  • 11,297
  • 18
  • 71
  • 121
  • possible duplicate of [Can I customize automatic event handler generation in Visual Studio?](http://stackoverflow.com/questions/4471593/can-i-customize-automatic-event-handler-generation-in-visual-studio) – Thomas Levesque May 06 '12 at 12:48
  • Both instructions have the same meaning. VS 11 beta uses the shorter form (see [this bug on Connect](https://connect.microsoft.com/VisualStudio/feedback/details/632300/auto-generated-event-handlers-should-use-implicit-conversion-of-method-group-to-delegate)) – Thomas Levesque May 06 '12 at 12:49
  • @Thomas Levesque: Doesn't really explain *why* the IDE does this... that would fit better as an answer here, unless there's another question asking why... – BoltClock May 06 '12 at 12:49
  • 4
    I'm guessing that this went unchanged until VS11 because it "just worked". That said, I very much welcome it being changed in VS11. – BoltClock May 06 '12 at 12:50
  • 2
    The 2 lines are the same except that the short form does not work in C# 1.x. I assume there is/was a concern to implement features for the widest range of versions. – H H May 06 '12 at 13:03

1 Answers1

4

Statement syntax is pretty subjective and you can't argue about taste. But notable is that both forms of the statement are syntax sugar in C#. The chronic problem with sugar is that it produces rotten teeth. This kind of syntax hides what is really going on and that forever get C# programmers in trouble.

The short form first, it completely obfuscates that the compiler actually creates a delegate object under the hood. Getting objects created is not really ever anything you want to hide under a floor mat, as much as you like to trust the garbage collector to always get it right. Unfortunately it doesn't, this gets programmers in trouble that use pinvoke that requires a function callback. The classic case is SetWindowsHookEx(), it uses a callback to let Windows deliver the notifications for hooked events. Which is very commonly written like this:

hhook = SetWindowsHookEx(WH_KEYBOARD_LL, hookProc, hInstance, 0);

Where hookProc is the callback method. That doesn't work, it is guaranteed to crash the program after a few seconds. The C# compiler automatically creates the delegate object to allow hookProc to be called and passes it to the SetWindowsHookEx function. Trouble is, there is no live reference to that object anywhere. The garbage collector cannot see that the native code is using that object. So the next garbage sweep destroys the object and that's a Big Kaboom when Windows makes the callback later. At least with the long form, there are some odds that the C# programmer realizes he'd better store the object reference somewhere else.

The long form is trouble too, it isn't long enough. It completely obfuscates that the C# compiler actually creates a delegate object that uses the this reference. Which happens when the delegate target is an instance method. This goes wrong when a C# programmer writes code like this:

        SystemEvents.UserPreferenceChanged += 
           new UserPreferenceChangedEventHandler(repaintControls);

Where repaintControls is an instance method of a form that repaints all the controls when the user changed the theme. What goes wrong here is that the delegate object captures the value of this and assigns it to a static event. Which forever leaves the form object referenced, unless you explicitly write code to unregister the event handler again. This requirement is just not obvious, certainly not from the sugar and otherwise a very uncommon requirement in Winforms programming. It is however a very nasty memory leak bug that can easily make your program fall over on an out-of-memory exception when it runs long enough.

Well, neither syntax is perfect and the C# language doesn't permit the full syntax like C++/CLI does that explicitly assigns the delegate target's object. The IDE team had to pick between a rock and a hard place. They picked the rock. VS11 will have the hard place, no doubt inspired by popular demand.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • "This requirement is just not obvious" -- eh, it kinda is. The += operator implies a -= operator, which also mentioned in most intro texts about event handlers. The remaining problems exist elsewhere in the language and all stem from the issue of holding on to, or not holding on to, references outside of the current object. It's a general memory management issue and I don't think the sugar really makes it any worse (or better, for that matter). – siride May 06 '12 at 15:34
  • Hmm, show me a Winforms or WPF programmer that writes a -= statement for *every* += statement and I'd be looking at a victim of OCD. The need to explicitly unsubscribe is learned first by rote, only understood much later. – Hans Passant May 06 '12 at 15:46
  • I won't account for people's stupidity or lack of fastidiousness. For my part, I do take care to include -= statements where it matters. Of course, I came from a C background, so caring about memory management is something that's part of my baseline thinking, even in C#. – siride May 06 '12 at 15:56