11

I'm trying to create a deck of cards by iterating over the enums Suit and Rank (I know there's no great way to iterate over enums but I don't see an alternative). I did this by adding an enumerator enum_count to the end of each enum, whose value is meant to represent the length and end of the enum.

#include <vector>

using namespace std;

enum class Suit: int {clubs, diamonds, hearts, spades, enum_count};
enum class Rank: int {one, two, three, four, five, six, seven, eight,
                nine, ten, jack, queen, king, ace, enum_count};

struct Card {
    Suit suit;
    Rank rank;
};

class Deck{
    vector<Card> cards{};
    public:
        Deck();
};

Deck::Deck() {
    // ERROR ON THE BELOW LINE
    for (Suit suit = Suit::clubs; suit < Suit::enum_count; suit++) {
        for (Rank rank = Rank::one; rank < Rank::enum_count; rank++) {
            Card created_card;
            created_card.suit = suit;
            created_card.rank = rank;
            cards.push_back(created_card);
        };
    };
};

However, when I try to loop over the enum, the compiler doesn't like that I'm trying to increment the suit++ and rank++ in the for-loop, stating:

card.cpp|24|error: no ‘operator++(int)’ declared for postfix ‘++’ [-fpermissive]|
card.cpp|25|error: no ‘operator++(int)’ declared for postfix ‘++’ [-fpermissive]|

What is the best way to go about creating a deck of cards without throwing away the useful enum data structures?

DBedrenko
  • 4,871
  • 4
  • 38
  • 73
  • 1
    [Related?](http://stackoverflow.com/questions/261963/how-can-i-iterate-over-an-enum) – Weak to Enuma Elish Feb 10 '16 at 10:48
  • 1
    @JamesRoot I tried to replicate many of the solutions proposed in that question (even though most answers there are about unscoped enums) and others I found on the internet, but to no avail. So I posted here my best attempt. – DBedrenko Feb 10 '16 at 10:50
  • 1
    @NewWorld I asked [something similar](http://stackoverflow.com/questions/13971544/using-enum-in-loops-and-value-consistency) some time ago. – PaperBirdMaster Feb 10 '16 at 13:46

6 Answers6

9

I would recommend doing something different. Create a vector of Suit and one to Rank, and loop over them using the power of STL

const std::vector<Suit> v_suit {Suit::clubs, Suit::diamonds, Suit::hearts, Suit::spades};

const std::vector<Rank> v_rank {Rank::one, Rank::two, Rank::three, Rank::four, Rank::five, 
                          Rank::six, Rank::seven, Rank::eight, Rank::nine, Rank::ten, Rank::jack, 
                          Rank::queen, Rank::king, Rank::ace};

Yes, you have to type them twice, but this permits you to use whatever values you want for them (ie. not consecutive), not use awkward stuff like enum_count (What card do you want? Give me a diamonds enum_count!!), no need for casting, and use the iterators provided to std::vector.

To use them:

for(const auto & s : v_suit)
    for (const auto & r : v_rank)
        cards.push_back({s,r});
hlscalon
  • 7,304
  • 4
  • 33
  • 40
  • Thanks for helping me :) I like this solution the best so far; it is the cleanest, and the only ugly part is that the enum and vector have to be kept in sync manually by the programmer if ever one of them changes, but I think this really is acceptable. – DBedrenko Feb 10 '16 at 12:02
  • Wouldn't it be better to make the vectors const? I mean, noone should ever alter them at runtime. – Simon Kraemer Feb 10 '16 at 14:03
  • I implemented this way and it works great, but may I know why you pass references (`&`) in the for-loops? The deck is created just as well without passing references. – DBedrenko Feb 10 '16 at 14:44
  • @SimonKraemer Yes, I was even forced to do this, because if I put these vector declarations in a header file and include this file, I would get a compiler error `multiple definitions of 'ranks'` for reasons unknown to me. – DBedrenko Feb 10 '16 at 14:46
  • @SimonKraemer Yes, it would. Edited – hlscalon Feb 10 '16 at 15:40
  • 1
    @NewWorld It's good to use const whenever is possible, and this is one of the case as s and r aren't modified. See [const correctness](https://isocpp.org/wiki/faq/const-correctness) and [sell-me-on-const-correctness](http://stackoverflow.com/questions/136880/sell-me-on-const-correctness) – hlscalon Feb 10 '16 at 15:40
  • @old_mountain Thanks for the links about const, but may I know why you chose to pass references (`&`) in the for-loops in your answer? The deck is created fine without passing references. – DBedrenko Feb 10 '16 at 21:08
  • @NewWorld It's to avoid making copies (this case is not so relevant since copying ints is cheap). See [this](http://stackoverflow.com/a/15176177/3658660) and [this](http://stackoverflow.com/questions/17032267/c11-range-based-for-loop-efficiency-const-auto-i-versus-auto-i) – hlscalon Feb 11 '16 at 10:48
  • Vector is a strange choice here. Use std::array or Suit[enum_count] instead. The containers will never change size at runtime, as that would make no sense. It is also bad practice to use excessive dynamic allocation in C++. – pro-gramer Aug 25 '20 at 19:57
4

You could cast your suit and rank variables to an int& and increase them as such.

    for (Suit suit = Suit::clubs; suit < Suit::enum_count; ((int&)suit)++) {
        for (Rank rank = Rank::one; rank < Rank::enum_count; ((int&)rank)++) {

Yet this might cause some problems like when you assign values to your enum entries.


You could also create a little function that does this for you with the correct type:

template <typename T>
T& increment(T& value)
{
    static_assert(std::is_integral<std::underlying_type_t<T>>::value, "Can't increment value");
    ((std::underlying_type_t<T>&)value)++;
    return value;
}

Deck::Deck() {
    bool a = std::is_integral<std::underlying_type_t<Suit>>::value;

    // ERROR ON THE BELOW LINE
    for (Suit suit = Suit::clubs; suit < Suit::enum_count; increment(suit)) {
        for (Rank rank = Rank::one; rank < Rank::enum_count; increment(rank)) {
            Card created_card;
            created_card.suit = suit;
            created_card.rank = rank;
            cards.push_back(created_card);
        };
    };
};
Simon Kraemer
  • 5,700
  • 1
  • 19
  • 49
  • Thanks for the interesting solution. What does `(int&)` do here exactly? I know `int& some_arg` can be used in function parameter definitions to pass-by-reference. – DBedrenko Feb 10 '16 at 11:05
  • 1
    `int&` is a reference to a variable of type `int`. So when you change the reference you change the value variable the reference references. `((int&)suit)++` could also be written like `int& temp = (int&)suit; temp++;`. The only reason this works is that `Suit` and `Rank` "inherit" int, so that this cast is safe. – Simon Kraemer Feb 10 '16 at 11:11
  • @Simon Make it generic and you're golden. – LogicStuff Feb 10 '16 at 11:20
  • @LogicStuff Updated my answer – Simon Kraemer Feb 10 '16 at 12:50
4

With C++11, you can use a range-based for loop. All you need is to define an iterator class with operator++, operator!=, and operator* and to define begin or end as member functions or free functions.

Here is an example using an EnumRange class that includes an Iterator class and begin or end member functions. The class assumes that T is an enum class with contiguous values that start at 0 and end at MAX. The MAX declaration is used to avoid adding an invalid value, such as enum_count, to the enum. If the enum class does not define MAX, then the code will not compile.

template <class T>
struct EnumRange {
  struct Iterator {
    explicit Iterator(int v) : value(v) {}
    void operator++() { ++value; }
    bool operator!=(Iterator rhs) { return value != rhs.value; }
    T operator*() const { return static_cast<T>(value); }

    int value = 0;
  };

  Iterator begin() const { return Iterator(0); }
  Iterator end() const { return Iterator(static_cast<int>(T::MAX) + 1); }
};

This allows you to simply write:

enum class Suit {clubs, diamonds, hearts, spades, MAX=spades};
enum class Rank {one, two, three, four, five, six, seven, eight,
                 nine, ten, jack, queen, king, ace, MAX=ace};

for(const Suit s : EnumRange<Suit>())
    for (const Rank r : EnumRange<Rank>())
        cards.push_back({s,r});

An advantage of this approach is that it avoids the need to define/allocate a map or vector each time you want to iterator over an enum. Instead, the EnumRange::Iterator class stores a single integer and any changes to the enum are automatically supported. Also, since we have defined operator* to cast the integer to the enum type T, we know that the variable type of the range-based for loop is the enum.

All together, the result is the easy-to-read expression for(MyEnum s : EnumRange<MyEnum>()).

eric
  • 642
  • 1
  • 9
  • 11
3

You can not use this with enum class. You have to use the old style enum.

If you insist you use them. I can suggest you a bad solution not to use (unless you won't tell anyone that I suggested it):

for (Rank rank = Rank::one; 
     static_cast<int>(rank) < static_cast<int>(Rank::enum_count); 
     rank = static_cast<Rank>(static_cast<int>(rank)+1)){
};
Humam Helfawi
  • 19,566
  • 15
  • 85
  • 160
  • Thank you for the answer, could you tell me why using `static_cast` like this is "bad"? – DBedrenko Feb 10 '16 at 10:55
  • not static_cast .. the whole solution is dummy. I ask a similar question before to get rid of this for and there was no much option. I think you should reconsider the whole design to find a better way. – Humam Helfawi Feb 10 '16 at 10:57
  • Hmm then what's wrong with the solution? I am looking for a better design, but an Enum class seems the perfect way to model card suits and ranks... – DBedrenko Feb 10 '16 at 10:59
2

Additional answer in response to old_mountain's answer:

You can in some cases prevent that you forget to add new values to your list by using fixed arrays. The main problem with this is that the initializer accepts less arguments than specified but you can work around this:

template<typename T, typename...Args>
struct first_type
{
    using type = T;
};

template<typename... Args> 
std::array<typename first_type<Args...>::type, sizeof...(Args)> make_array(Args&&... refs) 
{
    return std::array<typename first_type<Args...>::type, sizeof...(Args)>{ { std::forward<Args>(refs)... } };
}

I found the inspiration to make_array in this question, but modified it: How to emulate C array initialization "int arr[] = { e1, e2, e3, ... }" behaviour with std::array?

What it does is to use the first argument to find out what type the std::array shall be of and the number of arguments to get the real size. So the return type is std::array<firstType, numArgs>.

Now declare your lists like this:

const std::array<Suit, (size_t)Suit::enum_count> SuitValues = 
    make_array(Suit::clubs, Suit::diamonds, Suit::hearts, Suit::spades);

const std::array<Rank, (size_t)Rank::enum_count> RankValues = 
    make_array(Rank::one, Rank::two, Rank::three, Rank::four, 
               Rank::five, Rank::six, Rank::seven, Rank::eight,
               Rank::nine, Rank::ten, Rank::jack, Rank::queen, 
               Rank::king, Rank::ace);

When you add a value to the array your enum_count or whatever value you are using as delimiter will change and therefore the assignment from make_array will fail as the sizes of both std::arrays differ (which results in different types).

Example:

If you just add a new Suit, let's say hexa

enum class Suit : int { clubs, diamonds, hearts, spades, hexa, enum_count };

The compiler will fail with:

cannot convert from 'std::array<T,0x04>' to 'const std::array<Suit,0x05>'

I have to admit that I am not 100% happy with this solution as it requires a pretty ugly cast to size_t in the array declaration.

Community
  • 1
  • 1
Simon Kraemer
  • 5,700
  • 1
  • 19
  • 49
1

I also want to share my approach, based on a previous answer of mine which creates map<enum,string> using C++11 and C++14 features, the code is the one below:

// Shortcut to the map
template <typename ENUM>
using enum_map = std::map<ENUM, const std::string>;

// Template variable for each enumerated type
template <typename ENUM>
enum_map<ENUM> enum_values{};

// Empty function to end the initialize recursion
void initialize(){}

// Recursive template which initializes the enum map
template <typename ENUM, typename ... args>
void initialize(const ENUM value, const char *name, args ... tail)
{
    enum_values<ENUM>.emplace(value, name);
    initialize(tail ...);
}

With this template, we can change the Deck constructor this way:

Deck::Deck() {
    for (const auto &S : enum_values<Suit>) {
        for (const auto &R : enum_values<Rank>) {
            Card created_card;
            created_card.suit = S.first;
            created_card.rank = R.first;
            cards.push_back(created_card);
        };
    };
};

The only requirement in order to make the whole thing work is to call the initialize function this way:

initialize
(
    Suit::clubs,    "Clubs",
    Suit::diamonds, "Diamonds",
    Suit::hearts,   "Hearts",
    Suit::spades,   "Spades",

    Rank::one,   "1",
    Rank::two,   "2",
    Rank::three, "3",
    Rank::four,  "4",
    Rank::five,  "5",
    Rank::six,   "6",
    Rank::seven, "7",
    Rank::eight, "8",
    Rank::nine,  "9",
    Rank::ten,   "10",
    Rank::jack,  "J",
    Rank::queen, "Q",
    Rank::king,  "K",
    Rank::ace,   "A"
);

You can take a look at the Live example.

Community
  • 1
  • 1
PaperBirdMaster
  • 12,806
  • 9
  • 48
  • 94