0

Let's say I have a c# class which can be subscribed to like so:

public class Emitter {
  public delegate void EmitAction(Emitter emitter);
  public event EmitAction OnEmitted;

  public void DoEmit() {
     if(OnEmitted != null)
       OnEmitted(this);
  }
}

...and another class that makes use of it:

public class EmissionUser {

   public void SomeFunction() {
      Emitter em = new Emitter();
      em.OnEmitted += HandleIt;
      em.DoEmit();
   }

   public void HandleIt(Emitter em) {
      //I could just do em -= HandleIt, but is there any point?
   }
}

I know that for code cleanliness and avoiding memory leaks, it's always good have one unsubscribe for every subscribe when using events. However in a case like this, the Emitter instance is created locally in SomeFunction(), and not held in memory for too long, so I'm guessing the event subscription is destroyed with the local instance.

Is it still a good idea to unsubscribe in the handler? If so why?

balthatrix
  • 115
  • 6

2 Answers2

0

Yes you don't need to unsubscribe because as you said the em variable destroyed after the SomeFuction returned, and nobody else has reference to this object so the garbage collector can clean it up.

Tóth Tibor
  • 1,526
  • 12
  • 21
0

Well, em is a local instance and will get destroyed with the end of HandleIt. You can choose to leave it like that if you are not going to use it anywhere else.

But if you have the reference of it somewhere else, or going to create one in future, it would create problems. Also, finding root cause of bugs related to events is tricky.

I noticed that you are passing the emitter reference. Someone can again use the same reference to call different method or raise an event within handleIt.

So, I would suggest you to do it, just for the sake of future-proofing the code.

Prajwal
  • 3,930
  • 5
  • 24
  • 50