4
\$\begingroup\$

I have a number of custom events that a class might or might not subscribe to.

Action<Game1, MoveEventArgs> MoveEvent;
Action<Game1, InitializeEventArgs> InitializeEvent;
Action<Game1, DrawEventArgs> DrawEvent;
Action<Game1, LoadContentEventArgs> LoadContentEvent;

Each event is currently very similar just containing a number of properties such as:

public class MoveEventArgs : EventArgs
{
    public Vector2 Direction { get; set; }
}

public class LoadContentEventArgs : EventArgs
{
    public ContentManager Content { get; set; }
}

Some have more some have less. These are likely to be either expanded on or changed in the future if I need to pass more information.

A method subscribing to these events will generally access these arguments using them in someway that is specific to that event. For example a move event may look like this.

public void Move(object o, MoveEventArgs e)
{
    spriteInfo.Position = new Vector2(
        spriteInfo.Position.X + e.Direction.X, spriteInfo.Position.Y + e.Direction.Y
    );
}

And a content event may look like this.

public virtual void LoadContent(object o, LoadContentEventArgs e)
{
    spriteInfo.Texture = e.Content.Load<Texture2D>(spriteInfo.TextureName);
}

I would like to bundle these in someway so I can easily pass them into other functions. Such as in a list or dictionary.

I have come up with two solutions. The first is to change my event declarations from:

Action<Game1, InitializeEventArgs> InitializeEvent;
public virtual void Initialize(object o, InitializeEventArgs e) { }

to:

Action<Game1, EventArgs> InitializeEvent;
public virtual void Initialize(object o, EventArgs e) 
{ 
    InitializeEventArgs initializeEventArgs = (InitializeEventArgs)e; 
}

This will allow me to create data stuctures using the type Action<Game1, EventArgs> but will mean I lose the typing on my event subscriber methods possibly resulting in cast exceptions.

The second is creating a struct with my Events in which I can pass to classes/methods.

public struct Events
{
    public Action<Game1, MoveEventArgs> MoveEvent { get; set; }
    public Action<Game1, InitializeEventArgs> InitializeEvent { get; set; }
    public Action<Game1, DrawEventArgs> DrawEvent { get; set; }
    public Action<Game1, LoadContentEventArgs> LoadContentEvent { get; set; }
}

Which I can then use to subscribe to the event in the appropriate class/method using:

Events.InitializeEvent += Initialize;

This seems good but it could potentially become a very large struct especially if I were to have a large number of events.

So I guess my main question is does anyone have improvements for these ideas or something different?

\$\endgroup\$

closed as off-topic by yuri, Toby Speight, Peter Taylor, t3chb0t, pacmaninbw Jun 12 at 12:21

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – yuri, Toby Speight, Peter Taylor, t3chb0t, pacmaninbw
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 5
    \$\begingroup\$ Welcome to Code Review. Please provide more concrete code, so that we can fully understand your situation and give you the proper advice. Specifically, what do these args look like, and how are they used? \$\endgroup\$ – 200_success Jun 10 at 1:26
  • \$\begingroup\$ I have added some more information about what the args look like and some of my methods that use them. If you feel that this still isn't enough please let me know what else you feel is missing. \$\endgroup\$ – Antony Capper Jun 10 at 6:38
  • \$\begingroup\$ I have rolled back your last edit. Please see What should I do when someone answers my question?: Do not add an improved version of the code after receiving an answer. Including revised versions of the code makes the question confusing, especially if someone later reviews the newer code. \$\endgroup\$ – t3chb0t Jun 11 at 18:03
  • \$\begingroup\$ @AntonyCapper You can always post a self-answer with your conclusions. \$\endgroup\$ – dfhwze Jun 11 at 20:26
1
\$\begingroup\$

Review Solution 1

Sacrificing usability for the common consumer of your API in order to gain some usability for specific consumers that happen to subscribe on a multitude of events, should never be considered best practice.

Action<Game1, EventArgs> InitializeEvent;
public virtual void Initialize(object o, EventArgs e) 
{ 
    // the average consumer does not want to cast here 
    // if he only wants to be subscribed to this event ->
    InitializeEventArgs initializeEventArgs = (InitializeEventArgs)e; 
}

Review Solution 2

Grouping events together is a better approach, since the common consumer is not impacted. It is an augmentation, rather than a substitution, of the event pattern. Though your implementation using a struct is viable, I would opt to use a more common approach - the Callback Interface.

public struct Events
{
    public Action<Game1, MoveEventArgs> MoveEvent { get; set; }
    public Action<Game1, InitializeEventArgs> InitializeEvent { get; set; }
    public Action<Game1, DrawEventArgs> DrawEvent { get; set; }
    public Action<Game1, LoadContentEventArgs> LoadContentEvent { get; set; }
}

Proposed Solution

You should determine whether to use either events, a callback interface or a mixture of both.

Let's assume you would like consumers to be able to subscribe to single events, and allow specific consumers to subscribe to all of them. The mixture of events and a callback interface could be implemented as follows.

Create event arguments

public class MoveEventArgs : EventArgs { /* .. impl */ }
public class InitializeEventArgs : EventArgs { /* .. impl */ }
public class DrawEventArgs : EventArgs { /* .. impl */ }
public class LoadContentEventArgs : EventArgs { /* .. impl */ }

Create a callback interface that bundles handlers for all the relevant events.

 public interface IGameListener
 {
      void OnMove(Game1 game, MoveEventArgs e);
      void OnInitialize(Game1 game, InitializeEventArgs e);
      void OnDraw(Game1 game, DrawEventArgs e);
      void OnLoadContent(Game1 game, LoadContentEventArgs e);
 }

As discussed, you may want to create an adapter. This allows derived classes to decide which event handlers to override.

public abstract class GameAdapter : IGameListener
{
      public virtual void OnMove(Game1 game, MoveEventArgs e) {}
      public virtual void OnInitialize(Game1 game, InitializeEventArgs e) {}
      public virtual void OnDraw(Game1 game, DrawEventArgs e) {}
      public virtual void OnLoadContent(Game1 game, LoadContentEventArgs e) {}
}

Game1 uses events and allows for registration of specific listeners. If you don't like to 'pollute' the game with these actions, you could let a mediator perform the registration of the callback interface. Argument checks are omitted for brevity.

 public class Game1
 {
        public event Action<Game1, MoveEventArgs> MoveEvent;
        public event Action<Game1, InitializeEventArgs> InitializeEvent;
        public event Action<Game1, DrawEventArgs> DrawEvent;
        public event Action<Game1, LoadContentEventArgs> LoadContentEvent;

        public void Register(IGameListener listener)
        {
            MoveEvent += listener.OnMove;
            InitializeEvent += listener.OnInitialize;
            DrawEvent += listener.OnDraw;
            LoadContentEvent += listener.OnLoadContent;
        }

        public void Unregister(IGameListener listener)
        {
            MoveEvent -= listener.OnMove;
            InitializeEvent -= listener.OnInitialize;
            DrawEvent -= listener.OnDraw;
            LoadContentEvent -= listener.OnLoadContent;
        }

        // other instance code ..
 }
\$\endgroup\$
  • \$\begingroup\$ Your interface solution does for the most part solve my problem. The main piece of functionality missing is the ability to not have all the Events eg. if I want one class to have all but MoveEvent. Using your solution I would have to create an Interface for each combination as well as register methods for each new Interface. \$\endgroup\$ – Antony Capper Jun 10 at 17:02
  • 1
    \$\begingroup\$ @Antony This is a famous problem. Java has built-in framework listeners and adapters to solve this for many of their API's. You could do the same in C#. javatpoint.com/java-adapter-classes \$\endgroup\$ – dfhwze Jun 10 at 17:03

Not the answer you're looking for? Browse other questions tagged or ask your own question.