Is it good practice to use unique_ptr as an alternative to my own move constructors?

user_1287554 0
Viewed 210

I'm making a card game using C++. It's similar to solitaire, i.e. there are different stacks and piles and the cards are moved from one to another.

I'm using std::vector for the various piles, but instead of std::vector<Card>, I'm using std::vector<std::unique_ptr<Card>>. My rationale for doing this is that:

  1. It ensures that the ownership responsibility is with the vector that it is currently in.
  2. It makes sure that I never accidentally have more than 1 copy of the same card when transferring between stacks and piles, i.e. forces me to transfer using std::move.
  3. I don't have to implement my own copy and move constructors for Card.

Is this a reasonable way to use unique_ptr?

EDIT: Forgot to mention, there are different types of Cards which are derived from a base class. I'm a beginner and didn't want to muck around with different copy and move constructors at all levels.

c++
1 Answers
  1. It ensures that the ownership responsibility is with the vector that it is currently in.

This makes little sense to me. Vector owns all its elements, so unique pointer adds nothing of value in this regard.

  1. It makes sure that I never accidentally have more than 1 copy of the same card when transferring between stacks and piles, i.e. forces me to transfer using std::move.

If preventing copying is useful, then another alternative would be to make Card non-copyable - or keep Card copyable and create a non-copyable wrapper. In a way, std::unique_ptr<Card> could be seen as such wrapper, but it is more than just a wrapper and unnecessarily inefficient and complicated for that use case. A simple example:

struct UniqueCard : Card {
    using Card::Card;

    UniqueCard(UniqueCard&&) = default;
    UniqueCard& operator=(UniqueCard&&) = default;

    UniqueCard(const UniqueCard&) = delete;
    UniqueCard& operator=(const UniqueCard&) = delete;
};

Note that this design implies that cards must have some "empty" state, that they will be left in after move. This corresponds to having a null pointer in the stack that would be the case when moving from a unique pointer.

I'm not convinced that this is necessarily better than just allowing copying and leaving the responsibility to the user of the class to follow the rules of the game.

Is this a reasonable way to use unique_ptr?

Preventing copying alone is not a reasonable reason to use a unique pointer.

A different design that I would recommend trying out: Instead of storing the cards in vectors, represent the card stacks with linked lists. To move from stack to another, extract the node and splice into another list. This way the cards can be immutable, don't need to represent "empty" card, and you don't end up with duplicates.

I forgot to mention that there are different types of Cards which are derived from a base class

You cannot store derived class instances in a vector that contains base class objects. Storing derived instances is a use case where std::vector<std::unique_ptr<Card>> is reasonable, because a pointer can point to the base class sub object of the derived instances.

Note that dynamic polymorphism is not necessarily the best solution for a card game.

user_20793030
Related