5
\$\begingroup\$

I finished my Blackjack OO game and I'd like to get my OO skills reviewed.

#include <iostream>
#include <vector>
#include <algorithm> 
#include <ctime>
#include <string>

class Card
{
    private:      
    char m_card;         //  'A'   ,'2', '3', '4', '5', '6', '7', '8', '9', 'T', 'J', 'Q'
    int m_value,         //1 or 11 , 2  , 3  ,  4,   5 ,  6 ,  7 ,  8 ,  9,  10, 10,  10
    m_suite;             //0 spades, 1 clubs, 2 hearts, 3 diamonds
    std::string m_nameS; 
    bool m_given;        //Check if it's been wether given or not. works as an index

    int assignValue();   

    public:
    Card(char, int);
    Card(const Card&);

    int getValue()        const;
    int getSuite()        const;
    std::string getName() const;
    bool getIfGiven()     const;
    char getCard()        const;

    void setGiven(bool x);

    void nameCard();
};

int Card::assignValue()
{
    if(m_card == 'A')         return 11;
    else if (m_card == 'T' || 
             m_card == 'J' || 
             m_card == 'Q')   return 10;
    else                      return (m_card - '0');
}

 Card::Card(char cardV, int suite)
{
    m_card = cardV;
    m_value = assignValue();
    m_suite = suite;
    m_given = false;
}

Card::Card(const Card& card)
{
    m_card = card.getCard()     ;
    m_value = card.getValue();
    m_suite = card.getSuite()   ; 
    m_nameS = card.getName()    ;
    m_given = card.getIfGiven() ; 
}

int Card::getValue() const
{ return m_value; }

int Card::getSuite() const
{ return m_suite; }

std::string Card::getName() const
{ return m_nameS; }

bool Card::getIfGiven() const
{ return m_given; }

char Card::getCard() const
{ return m_card; }

void Card::setGiven(bool x)
{ m_given = x; }



void Card::nameCard()
{
    switch(m_suite)
    {
        case 0: m_nameS = "spades"  ; break;
        case 1: m_nameS = "clubs"   ; break;
        case 2: m_nameS = "hearts"  ; break;
        case 3: m_nameS = "diamonds";
    }
    if(m_card == 'A')       std::cout << "Ace"  ;
    else if (m_card == 'J') std::cout << "Joker";
    else if (m_card == 'Q') std::cout << "Queen";
    else                    std::cout << m_value;

    std::cout << " of " << m_nameS << std::endl;
}

class Deck
{
    private:
    std::vector<Card*> m_deck;

    public:    
    Deck();

    void fillDeck();
    void shuffle() ;
    Card giveCard();
};

Deck::Deck()
{ 
    //Initialize the deck: 
    fillDeck();
    shuffle();
}

void Deck::fillDeck()
{
    for(int i = 1; i <= 12; ++i)
    {
        char c;
        switch(i){
            case 1 :  c = 'A'; break;
            case 2 :  c = '2'; break;
            case 3 :  c = '3'; break;
            case 4 :  c = '4'; break;
            case 5 :  c = '5'; break;
            case 6 :  c = '6'; break;
            case 7 :  c = '7'; break;
            case 8 :  c = '8'; break;
            case 9 :  c = '9'; break;
            case 10:  c = 'T'; break;
            case 11:  c = 'J'; break;
            case 12:  c = 'Q';
        }
        for(int n = 0; n <= 3; ++n)
        {
            m_deck.push_back(new Card(c,n));
        }
    }
}

void Deck::shuffle()
{
    std::random_shuffle(m_deck.begin(), m_deck.end());
}

Card Deck::giveCard()
{
    for(int i = 0; i < m_deck.size(); ++i)
    {
        if(!m_deck[i]->getIfGiven())
        {
            m_deck[i]->setGiven(1);
            return (*m_deck[i]);
        }
    }
}

class Hand
{

    private:
    std::vector<Card> m_hand;

    public:

    void init(Card, Card) ;
    void display()        ;
    void displayAsDealer();
    void addCard(Card)    ;

    int getNumOfCards();
    int getTotalVal()  ;
};

void Hand::init(Card card1, Card card2)
{
    m_hand.push_back(card1);
    m_hand.push_back(card2);
}

void Hand::display()
{
    for(std::vector<Card>::iterator iter = m_hand.begin();
    iter != m_hand.end(); (++iter))
    {
        iter->nameCard();
    }
}

void Hand::displayAsDealer()
{
    m_hand[0].nameCard();
}

void Hand::addCard(Card card)
{
    std::cout << "\nTook out: ";
    card.nameCard();

    m_hand.push_back(card); 
}

int Hand::getTotalVal()
{
    int sum = 0;
    for(std::vector<Card>::iterator iter = m_hand.begin();
    iter != m_hand.end(); (++iter))
    {
        sum += iter->getValue();
    }
    return sum;
}

int Hand::getNumOfCards()
{
    return m_hand.size();
}

class Player
{
    protected:
    Deck m_deck;
    Hand m_mano;
    bool m_canGetCards;
    int score;

    public:
    Player(Deck deck);
    void plant();
    void anotherCard();
    void seeCards();

    int getScore() const;

    bool perdio();
    bool ganoFR();
 };

 Player::Player(Deck deck)
 {
    m_canGetCards = 1;
    m_deck = deck;
    m_mano.init(deck.giveCard(), deck.giveCard());
 }

 void Player::plant()
 {
    m_canGetCards = 0;
    score = m_mano.getTotalVal();
 }

 void Player::anotherCard()
 {
    if(m_canGetCards) m_mano.addCard(m_deck.giveCard());
    else std::cout << "You cannot get more cards" << std::endl;
 }

 bool Player::perdio()
 {
    if(m_mano.getTotalVal() > 21)
    {
        std::cout << "Te pasaste de 21" << std::endl;
        return true;
    }
    return false;
 }

 bool Player::ganoFR()
 {
    return (m_mano.getTotalVal() == 21 && m_mano.getNumOfCards() == 2);
 }

 void Player::seeCards()
 {
    std::cout << "Your cards: ";
    m_mano.display();
 }

 int Player::getScore() const
 { return score; }

 class Dealer : public Player  //The dealer IS A player
 {
    public:
    using Player::Player;

    void seeCard()   ; 
    void game(Player);
 };

 void Dealer::seeCard()
 {
    std::cout << "Dealer's card: ";
    m_mano.displayAsDealer();
    std::cout << std::endl;
 }

 void Dealer::game(Player play)
 {
    while(m_mano.getTotalVal() <= 21)
    {
        std::cout << "Cards: ";
        m_mano.display();
        std::cout << std::endl;
        if(m_mano.getTotalVal() > play.getScore() && m_mano.getTotalVal() <= 21)
        {
            std::cout << "The dealer won because his score was greater than yours and it didn't exceed 21 in any form." << std::endl;
            return;
        }
        if(m_mano.getTotalVal() == play.getScore())
        {
            std::cout << "A tie! " << std::endl;
            return;
        }
        anotherCard();
    }
    if(m_mano.getTotalVal() > 21)
    {
        std::cout << "El dealer se paso de 21, el perdio";
        return;
    }
 }

 void introduct()
 {
    std::cout << "Controls: 1 for take another card" << std::endl;
    std::cout << "2 for planting" << std::endl;
    std::cout << "\n\n";
    std::cout << "Have Fun!! \n\n" << std::endl;
}

int main()
{
    srand(time(0));

    introduct();

    Deck deck; 
    Player yo(deck);
    Dealer casino(deck);
    char x;
    bool bbreak = 0;

    yo.seeCards();
    casino.seeCard();

    if(yo.ganoFR())
    {
        std::cout << "You won! " << std::endl;
    }
    else if (casino.ganoFR())
    {
        std::cout << "The dealer won!" << std::endl;
    }
    else
    {
        while(!yo.perdio())
        {
            std::cout << "What do you want to do: ";
            std::cin >> x;
            if(x == '1')
            {
                yo.anotherCard();
            }
            else if(x == '2')
            {
                yo.plant();
                casino.game(yo);
                break;
            }
            yo.seeCards();
            casino.seeCard();
        }
    }

    return 0;
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ There are no King cards? \$\endgroup\$ Jan 27 '15 at 7:11
  • 1
    \$\begingroup\$ you should replace random_shuffle with shuffle since it is deprecated in C++14. See meetingcpp.com/index.php/br/items/… \$\endgroup\$
    – AndersK
    Feb 6 '15 at 11:46
2
\$\begingroup\$

Lot of code but here are some comments.

class Card

Personally I put private parts of a class at the end of the class, those are implementation details. The public parts OTOH are more important for the user of the class. (IMHO)

Use scoped enum for the card suit, as it is now you use char so any char can be accidentally assigned.

assignValue

assignValue depends on the order of when the member variable m_card was defined, it would be better to make it static and take as argument the card otherwise this can be an error source in the future.

A std::map could have been used to hold the values of the cards e.g. map

constructor

you do not initialize all declared variables, std::string is initialized automatically to empty string but it is good nevertheless to show that you initialize it for clarity. one day you may change the type of the variable and forget to add it to the constructor

nameCard

you seem to have omitted the King.

your switch sets a string for 0-3 but forgetting to call this function will forget to define that variable, you could instead make sure the card has a textual suit by defining it in the constructor same time you assign m_suite.

again the mapping from value to some string could be held in a std::map both for suits and for the textual values like "Ace". that would make things like Deck::fillDeck smaller.

Deck

Use a smart pointer instead of a raw pointer to the cards, that you do not have to worry about memory leaks.

std::vector<std::unique_ptr<Card>> m_deck;

when you initialize the deck just do

m_deck.push_back( make_unique<Card>( c, n ) );

after that you can treat the pointers as any other pointer and you do not need to delete them.

giveCard

This function copies the card from the deck, which somehow seems a bit unintuitive, but ok that works i guess as well. Alternatively you could move the card to the players hand, then when you initialize a new deck just create 52 new cards in a fresh deck.

Hand

you may want to keep the hand sorted to make it easier for user.

display

There is no need to have the extra ( ) around ++iter

Player

You are inheriting from Player (Dealer) but Player has no virtual destructor, add one.

It is not necessary to write using Player::Player; in Dealer, when you inherit you can access the base class members that are protected/public. If you want to access private members then maybe you should reconsider why you made them private in the first place.

It seems wrong to declare the Deck to be a member variable in the Player class - do each player have their own marked deck? I would think it would be better to declare it outside Player/Dealer since they only have a handful of the cards at any time.

\$\endgroup\$
0
\$\begingroup\$

You'll want to decide if you're going to name your identifiers in Spanish or English and stick with one or the other instead of mixing them. Otherwise, only bilingual folks will be able to read your code.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.