C # - Static events on non-static classes

Question

There are situations when I really like static events, but the fact that I rarely see them in other people's code makes me wonder if I am missing something important. I found a lot of discussion about static events on this site, but most of them concern situations that are not interesting to me (for example, on static classes) or where I would not think to use them in the first place.

I'm interested in am - these are situations where I can have many examples of something and one instance of a long-lived “manager” who reacts to something in these cases. A very simple example to illustrate what I mean:

public class God { //the list of followers is really big and changes all the time, //it seems like a waste of time to //register/unregister events for each and every one... readonly List<Believer> Believers = new List<Believer>(); God() { //...so instead let have a static event and listen to that Believer.Prayed += this.Believer_Prayed; } void Believer_Prayed(Believer believer, string prayer) { //whatever } } public class Believer { public static event Action<Believer, string> Prayed; void Pray() { if (Prayed != null) { Prayed(this, "can i have stuff, please"); } } } 

For me, this seems like a much simpler and simpler solution than an instance event, and I also don't need to track changes in the collection of believers. In cases where the Believer class can “see” a class of type God, I could sometimes use the NotifyGodOfPrayer () method (which was the preferred answer in several similar questions), but often the class of the “Believer” type is in the “Model” - assembly, where I cannot or do not want to directly appeal to the class of God.

Are there any actual flaws in this approach?

Edit: Thanks to everyone who has already answered. My example may be bad, so I would like to clarify my question:

If I use such static events in situations where

• I am sure that there will be only one instance of the subscribing object
• which is guaranteed to exist as long as the application is running
• and the number of instances I'm looking at is huge.

that is, are there potential problems with this approach that I don’t know about?

If the answer to this question is yes, I really am not looking for alternative implementations, although I really appreciate everyone who is trying to be useful. I am not looking for the most beautiful solution (I would have to give this prize to my own version just to be short and easy to read and maintain :)

Answer 1

One important thing to know about events is that they call objects that are attached to the event so as not to collect garbage until the owner of the event is garbage collected or until the event handler is detached.

To give it as an example, if you have a polytheistic pantheon with many gods, where you promote and omit gods such as

 new God("Svarog"); new God("Svantevit"); new God("Perun"); 

the gods would remain in your RAM as long as they are tied to Believer.Prayed . This would cause your application to leak gods.

---

I will also comment on the design decision, but I understand that the example you made may not be the best copy of your real scenario.

It seems more reasonable to me not to create dependence on God to Believer , but to use events. A good approach would be to create an aggregator of events that will stand between believers and gods. For example:

 public interface IPrayerAggregator { void Pray(Believer believer, string prayer); void RegisterGod(God god); } // god does prayerAggregator.RegisterGod(this); // believer does prayerAggregator.Pray(this, "For the victory!"); 

When calling the Pray method, the event aggregator calls the corresponding method of the God class in turn. To manage links and avoid memory leaks, you can create an UnregisterGod method or keep the gods in a collection of weak links such as

 public class Priest : IPrayerAggregator { private List<WeakReference> _gods; public void Pray(Believer believer, string prayer) { foreach (WeakReference godRef in _gods) { God god = godRef.Target as God; if (god != null) god.SomeonePrayed(believer, prayer); else _gods.Remove(godRef); } } public void RegisterGod(God god) { _gods.Add(new WeakReference(god, false)); } } 

---

Quick Tip: Temporarily keep the event delegate as listeners can cancel their event handlers

 void Pray() { var handler = Prayed; if (handler != null) { handler(this, "can i have stuff, please"); } } 

---

Edit

Keeping in mind the details that you added about your scenario (a huge number of event counters, a constant and single-ended observer of events), I think you have chosen the right scenario, solely for reasons of efficiency. It creates the smallest memory and processor overhead. I would not use this approach at all, but for the scenario that you described, a static event is a very pragmatic decision that I can make.

One drawback that I see is the flow of control. If your event listener is created in one instance, as you say, I would use the singleton (anti) pattern and call the God method from Believer directly.

 God.Instance.Pray(this, "For the victory!"); //or godInstance.Pray(this, "For the victory!"); 

Why? Because then you get more competent control over the action of prayer. If you decide that you need to subclass Believer into a special kind that does not pray on certain days, then you would control it.

Answer 2

I really think that having an instance even will be cleaner and defiantly more readable.
It is much easier to view it, since the instance is the victim, so its prayer event receives a trigger. And I do not see ant flaws for this. I don’t think that monitor changes any more fussing than monitoring a static event. but this is the right way ...

List Control:
Change the list as an ObservableCollection (and see NotifyCollectionChangedEventArgs ).
Control it:

 public class God { readonly ObservableCollection<Believer> Believers = new ObservableCollection<Believer>(); public God() { Believers = new ObservableCollection<T>(); Believers.CollectionChanged += BelieversListChanged; } private void BelieversListChanged(object sender, NotifyCollectionChangedEventArgs args) { if ((e.Action == NotifyCollectionChangedAction.Remove || e.Action == NotifyCollectionChangedAction.Replace) && e.OldItems != null) { foreach (var oldItem in e.OldItems) { var bel= (Believer)e.oldItem; bel.Prayed -= Believer_Prayed; } } if((e.Action==NotifyCollectionChangedAction.Add || e.Action==NotifyCollectionChangedAction.Replace) && e.NewItems!=null) { foreach(var newItem in e.NewItems) { foreach (var oldItem in e.OldItems) { var bel= (Believer)e.newItem; bel.Prayed += Believer_Prayed; } } } } void Believer_Prayed(Believer believer, string prayer) { //whatever } }