tags:

views:

199

answers:

8

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?

+2  A: 

If getHand() returns by-value you're modifying a copy of the hand and not the original.

Adam Mitz
A: 

What is the declaration of getHand()? Is it returning a new Hand value, or is it returning a Hand& reference?

Greg Hewgill
+1  A: 

Can you post your code? If getHand() is not returning a reference you will be in trouble.

Mike Thompson
I posted the code.Changing it to Hand }fixed it.Thanks everyone!
Glad to be of help
Mike Thompson
A: 

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);
metao
+1  A: 

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.

metao
+1  A: 

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.

Ty
+1  A: 

Return a reference to the hand object eg.

Hand &Player::getHand() {
    return hand;
}

Now your addCard() function is operating on the correct object.

Adam Pierce
A: 

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.
Tyler