Question

I'm trying to design a class to represent a card I've decided to use enums to represent the rank and suit. However, I need to be able to take an input from the user and, unfortunately, it's not possible to take an enum directly from cin.

For this reason, I was intending to take in a string and use std::map to map each string to the enum value that it represents (as described in this question). I also didn't want my input to be case sensitive so I created a regex that should shift all characters into lowercase before matching.

The code I came up with is here:

istream& operator>>(istream& is, Card& d)
{
std::map<std::string,Card::Rank> mr;
std::map<std::string,Card::Suit> ms;
mr[std::regex("/two/i")] = Card::TWO;
mr[std::regex("/three/i")] = Card::THREE;
mr[std::regex("/two/i")] = Card::FOUR;
mr[std::regex("/two/i")] = Card::FIVE;
mr[std::regex("/two/i")] = Card::SIX;
mr[std::regex("/two/i")] = Card::SEVEN;
mr[std::regex("/two/i")] = Card::EIGHT;
mr[std::regex("/two/i")] = Card::NINE;
mr[std::regex("/two/i")] = Card::TEN;
mr[std::regex("/two/i")] = Card::JACK;
mr[std::regex("/two/i")] = Card::QUEEN;
mr[std::regex("/two/i")] = Card::KING;
ms[std::regex("/clubs/i")] = Card::CLUBS;
ms[std::regex("/diamonds/i")] = Card::DIAMONDS;
ms[std::regex("/hearts/i")] = Card::HEARTS;
ms[std::regex("/spades/i")] = Card::SPADES;

string srank, ssuit;
char c1;

if (is >> srank >> c1 >> ssuit)
{
    if (c1 == 'of')
    {
        Card::Rank rank = mr[srank];
        Card::Suit suit = ms[ssuit];
        d = Card(rank, suit);
    }
    else
    {
       is.clear(ios_base::failbit);
    }
}

return is;
}

However, on compile I receive this error:

error C2679: binary '[' : no operator found which takes a right-hand operand of type 'std::basic_regex<_Elem>' (or there is no acceptable conversion)
      with
      [
          _Elem=char
      ]
      c:\program files (x86)\microsoft visual studio 11.0\vc\include\map(173): could be 'Card::Rank &std::map<_Kty,_Ty>::operator [](std::basic_string<_Elem,_Traits,_Alloc> &&)'
      with
      [
          _Kty=std::string,
          _Ty=Card::Rank,
          _Elem=char,
          _Traits=std::char_traits<char>,
          _Alloc=std::allocator<char>
      ]
      c:\program files (x86)\microsoft visual studio 11.0\vc\include\map(190): or       'Card::Rank &std::map<_Kty,_Ty>::operator [](const std::basic_string<_Elem,_Traits,_Alloc> &)'
      with
      [
          _Kty=std::string,
          _Ty=Card::Rank,
          _Elem=char,
          _Traits=std::char_traits<char>,
          _Alloc=std::allocator<char>
      ]
      while trying to match the argument list '(std::map<_Kty,_Ty>, std::basic_regex<_Elem>)'
      with
      [
          _Kty=std::string,
          _Ty=Card::Rank
      ]
      and
      [
          _Elem=char
      ]

etc.

I've never used a map before and I'm not really understanding what I've done wrong; from the instructions I've managed to dig up, it seems to me that this should work.

Was it helpful?

Solution 2

Solution based on what you have:

std::regex is not std::string, these are completely different types. I also doubt, if you can get any sensible results, using std::regex as map key. What's more, std::map will never work as you expected. If you like to test a string against list of regexes and pick the one that matches, you have to do exactly that: iterate over all regexes and test each one. If you are going to laxer these regexes, it is what you'll need.

Much easier approach would be to convert all read strings to lowercase, and then using standard map of strings. Something like:

#include <cctype>

static inline void stringToLower(std::string &s) {
    for (uint i = 0; i < s.size(); ++i) {
        s[i] = tolower(s[i]);
    }
}

istream& operator>>(istream& is, Card& d)
{
    std::map<std::string,Card::Rank> mr;
    std::map<std::string,Card::Suit> ms;
    mr["two"] = Card::TWO;
    mr["three"] = Card::THREE;
    ...
    mr["king"] = Card::KING;

    ms["clubs"] = Card::CLUBS;
    ...

    string srank, ssuit, c1;

    if (is >> srank >> c1 >> ssuit)
    {
        stringToLower(c1);
        if (c1 == "of")
        {
            stringToLower(srank);
            stringToLower(ssuit);
            Card::Rank rank = mr[srank];
            Card::Suit suit = ms[ssuit];
            d = Card(rank, suit);
        }
        else
        {
            is.clear(ios_base::failbit);
        }
    }

    return is;
}

Note, that I changed c1 to string. In yout version, c1 would be the first character after srank (it would be some whiltespace). Also, char value can never be equal to 'of' ('of' is of type int). I changed it to "of" (double quotes), which is of type const char *. Remember, that it wouldn't work properly, if c1 was char*.

By the way, it is very innefficient to create mr and ms maps for every << call separately. Consider using some kind of singleton. Possible solution may be something like:

static std::map<std::string,Card::Rank> createmr() {
    std::map<std::string,Card::Rank> mr;
    mr["two"] = Card::TWO;
    mr["three"] = Card::THREE;
    ...
    mr["king"] = Card::KING;
    return mr;
}

static std::map<std::string,Card::Rank> createms() {
    std::map<std::string,Card::Rank> ms;
    ms["clubs"] = Card::CLUBS;
    ...
    return ms;
}

istream& operator>>(istream& is, Card& d)
{
    static std::map<std::string,Card::Rank> mr = createmr(); // note `static` keyword
    static std::map<std::string,Card::Suit> ms = createms();
    ...
}

Explanation: I used static keyword here, in 2 different kinds of places. One is before functin declaration. That static means, that this functions will be not available in other source files. I also used it in mr and ms declarations. There it means, that these variables are common for all calls of function, in which they were defined (in this case, operator>>). One consequence of that is, that createmr() and createms() will be called only once, in the first call of operator.

Possibly better way is to define mr and ms outside the function:

static std::map<std::string,Card::Rank> mr = createmr(); // note `static` keyword
static std::map<std::string,Card::Suit> ms = createms();

istream& operator>>(istream& is, Card& d)
{
    ...
}

static keyword here means, that these variables (which are now global), will be not accessible from outside this source file. It may be omitted here, but it is good practice to mark static all functions and global variables, that are not going to be used outside.

OTHER TIPS

You declared mr to be a map from string to rank. You're trying to use a regex as the key. But a regex is not a string. Pick one and then use it consistently.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top