Better alternative to static events

Question

I'm writing a simple game in Unity, and learning C# on my own. Currently I'm doing first pass on the scoring system. I decided to do this with native c# events. So my first idea was to have the Score class responsible for counting/keeping player score. This class would receive events from objects implementing the IScoreable interface.

It would look something like this (example code):

public interface IScoreable {
  event Action<IScoreable> Scored;
  int Value { get; set; }
}

public class Score { 
  public void addScore(IScoreable scoreable) { 
     //do something with scoreable.Value 
  }
}

In theory, it's decent enough, but I had a problem with coupling:

With just the above code, Score needs to know about all possible objects that implement IScoreable, so it can subscribe to the Scored event. Considering there will be a lot of various objects implementing this interface - it may get instantiated from different parts of code, I don't see any "clean" way of doing this.

Another option would be, to have every IScoreable object register itself with Score object, but this again would create strong coupling. For example adding another player, and subsequently another Score instance, would require rewriting all classes implementing IScoreable)

Which leaves me with two alternatives (that I see). Create some kind of event manager/ aggregator. This option is now for me (subjectively speaking, I like how in c# events are strongly connected to class that defines them).

Another option, the one I'm leaning towards, is to use a static event for this. This would require a switch from interface to abstract class for IScoreable. Which could be a big deal with c# single inheritance, but in this case it isn't (Unity component based approach, discourages deep inheritance trees) I think it would fit this use case quite well.

public abstract class Scorable {
  public static event Action<Scorable> Scored;
  protected virtual void onScored()  {  if (Scored != null) Scored(this);  }
  public int Value { get; set; }
}

Score object would simply subscribe to Scored, and be done. Every class inheriting from Scoreable would call base.onScore when needed, and be done. No additional classes would be needed. Other than possibility of memory leak - if not careful I don't see downsides to this.

But since the use of static events seems to be discouraged, I have my doubts, maybe because of my inexperience I don't see a simple solution.

So the question... Is there a better (cleaner, simpler) solution to my problem? Would you advise against the use of static event in this case?

Answer 1

I think you have this backwards. Rather than have a Score object that knows about everything that can change the score, you should have all of your objects that might update the Score know about a single instance (a singleton) Score object.

Really simple case might be something like:

 public class Score 
 {
     public int TheScore { get; private set; }

     public static _instance;
     public static Instance     // I'm using a static property, but you could also create a static "getInstance" method if you prefer
     {
         get 
         {
             if (_instance == null) 
             {
                 // Note: if you are multithreading, you might need some locking here to avoid ending up with more than one instance
                 _instance = new Score();
             }
             return _instance;
         }
     }

     private Score() 
     {
         // Note: it's important to have a private constructor so there is no way to make another instance of this object
     }

     public int AddToScore(int score)
     {
         // again - if you have multiple threads here you might need to be careful
         TheScore += score;
     }
 }

Now in your object that might update the score, you just:

 Score.Instance.AddToScore(100);     // adds 100 to the score

Now if you want to get really fancy here, you could abstract the Score class into an interface IScore and then use an inversion of control (IoC) container to manage it for you. That way you can decouple the implementation of the class that holds the score from the classes that will consume it.

And of course to get the current score:

 int currentScore = Score.Instance.TheScore;

If you want to use events, then you could look at the publisher/subscriber pattern where you'd basically have a singleton that acts as the manager for your events and everybody that need to publish an event (i.e. your Player class) will publish to it and your Score class will subscribe to it.

Answer 2

Here are a few changes that will give you much smoother coupling

 public interface IScoreable {
      event EventHandler<IScoreable> Scored;
      int Value { get; set; }
    }

Create a new ScoreEventArgs class to handle your event args in a more flexible way

public class ScoreEventArgs: EventArgs
{
  public int Score {get;set;}

  // Can put more properties here to include when the event is raised

  public ScoreEventArgs(int value)
  {
    this.Score=value;
  }
}

Changes to your abstract class to ensure that event is handled according to your needs

  public abstract class Scorable {

  event EventHandler<ScoreEventArgs> scored;
  public event EventHandler<ScoreEventArgs> Scored;
  {
      add { this.scored += value; }
      remove { this.Scored -= value; }
  }
  // onScored method now handles the object (could be player in your case) where its called from and the score

  protected virtual void onScored(IScorable sender,int score)  
  {  
      if (Scored != null)
      {
         var e = new ScoreEventArgs(score)
         Scored(sender,e);  
         // Could set optional this.Value += score; // to get the current score or something like that 
      }
  }
  public int Value { get; set; }
}

Your calling class which will implement the event and will have a score

public class Player : Scorable
{
     public Player()
     {
        // Also can register a scored event callback 
         Scored+= new EventHandler<ScoreEventArgs>(PlayerScored);
     }
     private void PlayerScored(object sender, ScoreEventArgs args)
     {
         // Other logic can go here as well, this method will be called after the onScored(this, e) is called from the PlayerScored method.. You can update your UI or do other stuff here
     }
     public event EventHandler<ScoreEventArgs> Scored
     {
          add {this.Scored+=value;}
          remove {this.Scored += value;}
     }
     private void PlayerScored()
     {
         // Raise the on scored event
         onScored(this, new ScoreEventArgs(10));
     }
}

Hope this clears some ambiguities..