Redundant code in composition class C++

Question

I'm trying to pick up C++. Everything was going well until my 'practice' program hit I very minor snag. That snag, I believe, stems from a design issue.

Think of Blackjack(21). I made a few classes.

1. Card
2. Deck
3. Hand
4. Player

A Deck consists of - for simplicities sake - Has An array of cards.
-It can show all it cards
-It can shuffle
-It can remove cards

A Hand Is A Deck -with the benefit of
-It can calculate its hand value
-It can add Cards to the hand

Now to get to my issue - the Player design

-A Player Has A hand (private access)
My problem with player, is that hand has a method function called addCardToHand. I feel a sense of redundancy/bad design if I have to create a Player method called addCardToHand(Card c) in which calls and passes to the same method in hand.

or

declare Hand h as a public accessible member and in 'main()' do something like
Player p;
Card aCard;
p.h.addCard(aCard);

Any advice would be enlightening and highly appreciated. Keep in mind I am learning.

Answer 1

The best answer here is: it depends :) I'll try to clarify it a little, though.

The first question is: does the Player class have any inner logic? If it's a simple container for Hand, I'd simply write Player.GetHand().AddCard(), because there is no reason to duplicate the code inside Player.AddCard() method, and the problem is solved.

Let's suppose now, that there is a need for implementing additional logic for adding a card to Player's hand. That means, that additional code in Player class has to be called while adding a card to Hand. In such case, I see three possible solutions.

(Sources only for demonstration purposes, may not compile)

• Restrict access to Hand, such that no one can retrieve it from Player. Player would have to implement methods like AddToHand, RemoveFromHand etc. Doable, but not comfortable to use.

  class Player
  {
  private:
      Hand hand;
  
  public:
      void AddToHand(Card & card) 
      { 
          hand.Add(card); 
      }
  };
  

• Use the observer pattern. When user (class user) calls Player.GetHand().AddCard(), Hand notifies Player, that data has changed and Player can act accordingly. You can achieve this quite easily using std::function from C++11 to implement events.

  class Deck
  {
  private:
      std::function<void(void)> cardsChanged;
  
  public:
      void Add(Card card)
      {
          // Add a card
          if (!(cardsChanged._Empty()))
              cardsChanged();
      }
  
      void SetCardsChangedHandler(std::function<void(void)> newHandler)
      {
          cardsChanged = newHandler;
      }
  };
  
  // (...)
  
  class Player
  {
  private:
      Hand hand;
  
      void CardsChanged() { ... }
  (...)
  public:
      Player()
      {
          hand.SetCardsChangedHandler([&this]() { this.CardsChanged(); } );               
      }
  };
  

• Define IHand interface with all necessary interface methods. Hand should obviously implement IHand and Player.GetHand() should return IHand. The trick is, that the IHand returned by Player do not necessarily have to be a Hand instance, but instead it can be a decorator acting as a bridge between user and real Hand instance (see decorator pattern).

  class IHand
  {
  public:
      virtual void Add(Card card) = 0;
      virtual void Remove(Card card) = 0;
  };
  
  class Hand : public IHand
  { 
      // Implementations
  }
  
  class PlayersHand : public IHand
  {
  private:
      Hand & hand;
      Player & player;
  
  public:
      PlayersHand(Hand & newHand, Player & newPlayer)
      {
          hand = newHand;
          player = newPlayer;
      }
  
      void Add(Card card)
      {
          hand.Add(card);
          player.HandChanged();
      }
  
      // ...
  };
  
  class Player
  {
  private:
      Hand hand;
      PlayersHand * playersHand;
  
  public:
      Player()
      {
          playersHand = new PlayersHand(hand, this);
      }
  
      IHand GetHand()
      {
          return playersHand;
      }
  }
  

Personally, In the second case, I would choose the second solution - it's quite straightforward and easy to extend and reuse in case of further needs.

Answer 2

Function call forwarding is a common practice. You should think about it as adding some level of abstraction. This is not exactly doing the same thing again (which redundancy would mean), but implementing one method, using another one.

You can imagine some modifications in the future, like adding Player's cards cache, or some other stuff that need to be updated when user call addCardToHand. Where would you add the cache-updating code if you didn't implement the forwarding method?

Also note, that the "interface" of Player::addCardToHand doesn't need to be identical with Card::addCard i.e. arguments and returned value can be different in these functions. Maybe in this case it's not so important, but generally the forwarding function is the place where some translation between Player's interface and Hand's interface may be added.