Is this dangerous? About events

Question

Maybe a stupid question, but..

In my code I use the following construction at several places.

void MyFunction() 
{
 DoSomething(myClass.myProperty)

 myClass.PropertyChanged += (s,e ) => {
   if (e.PropertyName == "myProperty") {
  DoSomething(myClass.myProperty);
   }
 }
}

So I want to do something initially, and also do the same when the property changes in the future.

Now the thing is, MyFunction() gets called several time during my program's execution. Will the delegate I assign to PropertyChanged be added evertime it passes through this method? (consuming more memory every iteration and slowing down the program) Or is the compiler/runtime smart enough to understand I should be only added the first time..? And if so, how does this work?

Answer 1

The compiler can't know your intent ... it will faithfully attach the event handler on each call to MyFunction().

Look at it this way - they compiler can't know that the reference variables (e.g. myclass) in your function refer to the same instances of objects you've attached handlers to previously. Nor can it know that you haven't detached the handler somewhere else between calls. It can't make assumptions like this.

You, however, could restructure your code so that the handler is only attached once. Since other consumers may subscribe to the PropertyChanged event, you need to keep some independent state around to know whether you have subscribed or not. For example:

if( !haveSubscribedToPropertyChanged ) {
    myClass.PropertyChanged += (s,e ) => {
          if (e.PropertyName == "myProperty") {
                DoSomething(myClass.myProperty);
          }
        }
        haveSubscribedToPropertyChanged = true;
      }

Answer 2

Many of the other answers suggest that you should check whether the PropertyChanged event is null to prevent adding several listeners. A problem with that solution is that PropertyChanged can be non-null if other parts of the code listens to the same event but for another property, like this.

void AnotherFunction() 
{
  myClass.PropertyChanged += (s,e) => {
    if (e.PropertyName == "anotherProperty") {
      DoSomethingElse(myClass.anotherProperty);
    }
  }
}

A better solution, in my opinion, would be to maintain a boolean flag which is initially false, and only set to true when MyFunction is called. Then, check this flag whether you need to add the event handler.