How to use one object's method to update another object's attribute?
Question
I have three (C++) classes: Player, Hand, and Card.
Player has a member, hand, that holds a Hand. It also has a method, getHand(), that returns the contents of hand.
Hand Player::getHand() {
return hand;
}
Hand has a method, addCard(Card c), that adds a card to the hand.
I want to do this:
player1.getHand().addCard(c);
but it doesn't work. It doesn't throw an error, so it's doing something. But if I examine the contents of player1's hand afterward, the card hasn't been added.
How can I get this to work?
Solution
If getHand() is not returning a reference you will be in trouble.
OTHER TIPS
If getHand() returns by-value you're modifying a copy of the hand and not the original.
A Player.addCardToHand() method is not unreasonable, if you have no reason to otherwise expose a Hand. This is probably ideal in some ways, as you can still provide copies of the Hand for win-checking comparisons, and no-one can modify them.
Your method needs to return a pointer or a refernce to the player's Hand object. You could then call it like "player1.getHand()->addCard(c)". Note that that is the syntax you'd use it it were a pointer.
Return a reference to the hand object eg.
Hand &Player::getHand() {
return hand;
}
Now your addCard() function is operating on the correct object.
What is the declaration of getHand()? Is it returning a new Hand value, or is it returning a Hand& reference?
As has been stated, you're probably modifying a copy instead of the original.
To prevent this kind of mistake, you can explicitly declare copy constructors and equals operators as private.
private:
Hand(const Hand& rhs);
Hand& operator=(const Hand& rhs);
getX() is often the name of an accessor function for member x, similar to your own usage. However, a "getX" accessor is also very often a read-only function, so that it may be surprising in other situations of your code base to see a call to "getX" that modifies X.
So I would suggest, instead of just using a reference as a return value, to actually modify the code design a bit. Some alternatives:
- Expose a getMutableHand method that returns a pointer (or reference). By returning a pointer, you strongly suggest that the caller uses the pointer notation, so that anyone reading the code sees that this variable is changing values, and is not read-only.
- Make Player a subclass of Hand, so that anything that manipulates a Hand also works directly on the Player. Intuitively, you could say that a Player is not a Hand, but functionally they have the right relationship - every Player has exactly one hand, and it seems that you do want to be able to have the same access to a Hand via Player as you would directly.
- Directly implement an addCard method for your Player class.