Subscribing to events in factory -- better place?

Question

I'm implementing an event-driven game in WPF and have the following Blackjack class:

public class Blackjack
{
    public Dealer Dealer { get; } = new Dealer();
    public Player Player { get; } = new Player();

    public IList<Player> AiPlayers { get; } = new List<Player>();

    public event EventHandler<EventArgs>? NewGameStarted;
    public event EventHandler<EventArgs>? HandStarted;
    public event EventHandler<GameOverEventArgs>? HandEnded;

    public Blackjack()
    {
        Dealer.Hand.Busted += DealerBusted;

        Player.BetPlaced += BetPlaced;
        Player.PlayerHit += PlayerHit;
        Player.PlayerStand += PlayerStand;
        Player.Hand.Blackjack += PlayerBlackjack;
        Player.Hand.Busted += PlayerBusted;
        _gameStates[Player] = GameStates.PLAYING;
    }

    public void StartNewGame(int numAiPlayers)
    {
        for (var i = 0; i < numAiPlayers; i++)
        {
            var ai = new Player();
            AiPlayers.Add(ai);
            ai.PlayerHit += PlayerHit;
            ai.PlayerStand += PlayerStand;
            ai.Hand.Blackjack += PlayerBlackjack;
            ai.Hand.Busted += PlayerBusted;

            _gameStates[ai] = GameStates.PLAYING;
        }

        OnNewGameStarted(new EventArgs());
    }

    // More methods
}

But as you can see, the player/ai creation is starting to pollute my constructor and StartNewGame method. Additionally, I want to be able to use different AIs to simulate different strategies.

I thought about moving this to a factory, but realized I would be subscribing to the events from Blackjack inside the factory. This feels odd to me... Is this a normal thing to do or is it best to just create methods in my Blackjack class that handle player/ai creation and wiring up the events.

Answer 1

The factory pattern would be useful if the Blackjack object could be build different ways. That doesn't seem the case here. You are concerned about the amount of event handlers that need to be mapped in the constructor. So, how about simplifying the events?

What I see is good, the event name contains context about the event and each event is mapped to it's handler one to one.

ai.PlayerHit += PlayerHit

What would happen if the name of the event became a property of a generic event that just represented something of interest had happened in the object.

ai.OnAction += OnAction

The event could contain information about the action that was taken; information that can be used to decide how the event should be processed; what you are coding in the event handlers become methods.

class ActionEvent  // information about the event
   enum PlayerAction

enum PlayerAction  // an enumeration containing the player actions
   Bet
   Hit
   Stand

Then you can have an if statement in a single event handler

if(e.PlayerAction = Bet)
   PlayBet();

That way you reduce all the events down to one, which is easier to configure in the constructor. It's like the factory idea you have is now about deciding what to do, rather than how to construct.

Hope that is helpful. Happy coding.

Answer 2

I'd probably do something like this instead:

   /// <summary>
   /// Immutable snapshot of a table that you
   /// can use to display to players or AIs.
   /// </summary>
   internal sealed class Table
   {
   }

   /// <summary>
   /// Represents a round of the game.
   /// </summary>
   internal sealed class Round
   {
       private readonly IEnumerable<Player> players;

       public Round(IEnumerable<Player> players)
       {
           this.players = players;
       }

       public void RunGame()
       {
           while (players.Any(t => !t.IsFinished))
           {
               foreach (var player in players.Where(t => !t.IsFinished).ToArray())
               {
                   var play = player.GetPlay(CurrentTable);
               }
           }
       }

       /// <summary>
       /// Returns a current snapshot of the table.
       /// </summary>
       private Table CurrentTable { get; }

       private void UpdateState(Player player, Play play)
       {
           //Update any state needed to make the table.
       }
   }

   /// <summary>
   /// Represents a player - implementations for different
   /// AIs and for the human player.
   /// </summary>
   internal abstract class Player
   {
       public abstract Play GetPlay(Table currentTableState);

       public abstract bool IsFinished { get; }
   }

   /// <summary>
   /// Represents the result of a play made by a player.
   /// </summary>
   internal sealed class Play
   {
       public bool IsBet { get; }
       public bool IsHit { get; }
       public bool IsStand { get; }
       public decimal Amount { get; }

       private Play(bool isBet, bool isHit, bool isStand, decimal amount)
       {
           IsBet = isBet;
           IsHit = isHit;
           IsStand = isStand;
           Amount = amount;
       }
   }

At that point, the game directs the players, not the other way around, so there is no need for the game to listen to the events. Any events would be an implementation detail for the human player implementation.