ok, I did the default constructor and called resize(). The assert is gone and the program runs.
I also did a little more playing, I added a third player 'Bob' who is also a CPU (not that it really matters right now).
Here's the output from my last run:
### HAND 1 ###
CPU's hand: Nine of Hearts, Ten of Spades, Jack of Hearts, Queen of Spades, King of Spades
Bob's hand: Ace of Spades, Eight of Spades, Jack of Diamonds, Jack of Spades, Queen of Hearts
Ludovico's hand: Eight of Hearts, Nine of Diamonds, Ten of Clubs, Jack of Clubs, Queen of Clubs
CPU has Straight, King high
Bob has One Pair, Jack
Ludovico has Straight, Queen high
### HAND 2 ###
Bob's hand: Ace of Hearts, Eight of Diamonds, Ten of Diamonds, Jack of Clubs, King of Clubs
Ludovico's hand: Eight of Clubs, Nine of Spades, Queen of Diamonds, Queen of Spades, King of Diamonds
CPU's hand: Ace of Diamonds, Ace of Clubs, Nine of Diamonds, Ten of Hearts, King of Hearts
Bob has High Card, Ace
Ludovico has One Pair, Queen
CPU has One Pair, Ace
### HAND 3 ###
Ludovico's hand: Ace of Hearts, Ace of Diamonds, Eight of Diamonds, Jack of Diamonds, Queen of Diamonds
CPU's hand: Nine of Diamonds, Ten of Spades, Jack of Spades, King of Diamonds, King of Clubs
Bob's hand: Eight of Hearts, Ten of Diamonds, Jack of Hearts, Jack of Clubs, King of Hearts
Ludovico has One Pair, Ace
CPU has One Pair, King
Bob has One Pair, Jack
### HAND 4 ###
CPU's hand: Ace of Clubs, Eight of Diamonds, Eight of Spades, Jack of Hearts, Queen of Hearts
Bob's hand: Nine of Diamonds, Ten of Hearts, Ten of Spades, Queen of Diamonds, King of Clubs
Ludovico's hand: Ace of Spades, Nine of Hearts, Nine of Clubs, Nine of Spades, King of Diamonds
CPU has One Pair, Eight
Bob has One Pair, Ten
Ludovico has Three Of A Kind, Nine
I don't know about you, but I'd almost like to see the hands in a short form, something like this:
### HAND 4 ###
CPU's hand: AC, 8D, 8S, JH, QH
Bob's hand: 9D, 10H, 10S, QD, KC
Ludovico's hand: AS, 9H, 9C, 9S, KD
CPU has One Pair, Eight
Bob has One Pair, Ten
Ludovico has Three Of A Kind, Nine
Also when I was going through fixing it so it would run again, I spent more time actually looking at the design. I have a couple of comments. First, although you can declare a lot of classes and/or methods as friends, it is generally not good design. If there is a motivating reason, go ahead, but for simple things you can avoid it.
As an example, card had 7 friends:
friend std::ostream& operator<<(std::ostream& lhs, card& obj);
friend bool suitCompare(card op1, card op2);
friend bool valueCompare(card op1, card op2);
friend class deck;
friend class hand;
friend class player;
friend class pokerGame;
almost all of those are friends so they can access the protected data members:
protected:
suit m_suit;
value m_value;
But you provided public accessors for those items, they're even inline so in theory you don't even pay the price of a call. (But you could change them later if you needed to and no-one would REALLY care.)
suit getSuit() { return m_suit; }
value getValue() { return m_value; }
I would recommend that you re-visit all of your friend relationships to determine if they are really adding value. (I still have the pokerPoints output operator friendship, he's pretty connected, but I'm still not sure about him.)
You also have at least 2 copies of the string arrays for the card names and the suit names. (One in card and one in game.) Also when people are using the arrays, they are always doing something like
strValue[foo.m_value - 1] (with the non-friendship it looks more like
strValue[foo.getValue()-1] . So I added a couple of static methods to card to do the lookup (and then I hid the arrays).
static std::string const & valueText(value v);
static std::string const & suitText(suit s);
These methods can be accessed like
card::valueText(foo.getValue()) and return a constant reference to the string from the hidden array.
Speaking of constant references, const can be your friend (sometimes he's a little painful at first, but you'll come to love him too). There were several places where you're passing literal copies of objects or passing non-constant references. (If you pass a non-constant reference, the function or method you call could change the object you pass them.) It's normally good form when you're writing a method that does not need to make changes to a parameter passed by reference to make it a const reference.
A side effect of this is that when you're working with const references, you can only call methods of the object that have been marked const. So, for example, I marked card's accessors (to get the suit and the value) as const because they will not change the card. So they could be called from someone that had a const reference to a card.
suit getSuit() const { return m_suit; }
value getValue() const { return m_value; }
The last thing (I remember) doing to card was moving the sort compare functions from being external friends to public static members of the class. (As members of the class, they get 'free' permission to access the protected data members.) Ok, so here's the class header:
#ifndef H_FRENCH_CARD
#define H_FRENCH_CARD
#include <iostream>
#include <string>
enum value { ACE = 1, TWO = 2,
THREE = 3, FOUR = 4,
FIVE = 5, SIX = 6,
SEVEN = 7, EIGHT = 8,
NINE = 9, TEN = 10,
JACK = 11, QUEEN = 12,
KING = 13
};
enum suit { HEARTS = 1, DIAMONDS = 2, CLUBS = 3, SPADES = 4 };
class card {
public:
card(value v, suit s);
card(const card &cpy);
card & operator=(const card &op2);
static std::string const & valueText(value v);
static std::string const & suitText(suit s);
static bool cmpSuit(card const & x, card const & y);
static bool cmpValue(card const & x, card const & y);
static bool cmpValueAceHigh(card const & x, card const & y);
suit getSuit() const { return m_suit; }
value getValue() const { return m_value; }
protected:
suit m_suit;
value m_value;
};
// Helper operator to output the 'text' version of a card
std::ostream& operator<<(std::ostream& lhs, card const & obj);
#endif // H_CARD
The
cmpValueAceHigh() comparison function could be used in hand sorting (oh, say from pokerPoints) to sort the hand by value and to get the ace to the end of the array. It would tend to make the hand outputs a little prettier and might allow you to be more generic in some of the 'if I have an ACE' code in pokerPoints. I was thinking about it, and wrote the comparison, but I haven't really looked into using it.
Here's the source to match the header:
#include "card.h"
const std::string strValues[13] = { "Ace", "Two", "Three",
"Four", "Five", "Six",
"Seven", "Eight", "Nine",
"Ten", "Jack", "Queen", "King" };
const std::string strSuits[4] = { "Hearts", "Diamonds", "Clubs", "Spades" };
std::string const & card::valueText(value v)
{
return strValues[static_cast<int>(v) - 1];
}
std::string const & card::suitText(suit s)
{
return strSuits[static_cast<int>(s) - 1];
}
bool card::cmpSuit(card const & x, card const & y)
{
return x.m_suit < y.m_suit;
}
bool card::cmpValue(card const & x, card const & y)
{
return x.m_value < y.m_value;
}
bool card::cmpValueAceHigh(card const & x, card const & y)
{
return x.m_value != ACE && x.m_value < y.m_value;
}
card::card(value v, suit s) {
m_value = v;
m_suit = s;
}
card::card(const card & cpy) {
m_value = cpy.m_value;
m_suit = cpy.m_suit;
}
card & card::operator=(const card &op2) {
m_value = op2.m_value;
m_suit = op2.m_suit;
return *this;
}
std::ostream& operator<<(std::ostream& lhs, card const & obj) {
lhs << card::valueText(obj.getValue()) << " of " << card::suitText(obj.getSuit());
return lhs;
}
note that to implement the above changes, there were ripples through almost all of the other project files. Fairly straightforward, but they were there.