Im having some problem in shuffling the deck. There are some repetition after it shuffle. Any solution for it>?

#include <iostream>
#include <windows.h>
#include "stdlib.h"
#include <cstdlib>
#include <ctime>
using namespace std;

const static int BLACK = 0;
const static int BLUE = 9;
const static int RED = 12;
const static int WHITE = 15;

class Card{

    public :
        Card();
        Card(char cardRank,char cardSuit);
        ~Card();

        char getCardSuit();
        char getCardRank();
        int getSolitaireValue();
        friend std::ostream & operator<< (std::ostream & os, Card& c);

    private :
        bool showCard;
        char cardRank, cardSuit;

};

Card::Card(){}
Card::Card(char r, char s) : cardRank(r), cardSuit(s), showCard(false){}
Card::~Card(){}

char Card::getCardSuit()
{
    return cardSuit;
}

char Card::getCardRank()
{
    return cardRank;
}

int Card::getSolitaireValue()
{
    if(cardRank == 'A')
        return 1;
    else if(cardRank == 'T')
        return 10;
    else if(cardRank == 'J')
        return 11;
    else if(cardRank == 'Q')
        return 12;
    else if(cardRank == 'K')
        return 13;
}

std::ostream & operator<< (std::ostream & os, Card& c)
{

    HANDLE handle = GetStdHandle(STD_OUTPUT_HANDLE);

    //make the suits colour blue and red
    if(c.cardSuit == char(5) || c.cardSuit == char(6))
    SetConsoleTextAttribute(handle, BLUE);
    else
    SetConsoleTextAttribute(handle,RED);

    // make frame white colour
    os<< c.cardSuit << c.cardRank;
    SetConsoleTextAttribute(handle,WHITE);


    return os;
}

const static int RANK_SIZE = 13;    // declare cardRank size to 13 (A-K)
const static int SUIT_SIZE = 4;     // declare cardSuit size to 4 (H,D,S,C)
const static char RANKS[] = {'A','2','3','4','5','6','7','8','9','T','J','Q','K'};
const static char SUITS[] = {char(3), char(4), char(5), char(6)}; //{'h','d','s','c'};

class Deck{
    private :
        Card cardDeck[52];
        int currentIndex;
    public :
        Deck();
        ~Deck();

        void populate();
        void shuffle();
        void printDeck();
};

Deck::Deck() : currentIndex(0)
{
    populate();
}

Deck::~Deck(){}

void Deck::populate()      //Populate column with cards
{
    int index = 0;
    for(int i=0; i<SUIT_SIZE; i++)
    {
        for(int j=0; j<RANK_SIZE;j++)
        {
            cardDeck[index] = Card(RANKS[j],SUITS[i]);
            index++;
        }
    }
}

void Deck::shuffle()
{
    int max = SUIT_SIZE * RANK_SIZE;
    for(int i=0; i<max; i++)
    {
        int randNum = rand() % 52;
        swap(cardDeck[i],cardDeck[randNum]);
        cout<<cardDeck[i]<<"   ";
    }
}

int main()
{
 Deck deck = Deck();
 deck.shuffle();
}

Sorry. inside the Card.cpp i copied twice the function

Hi,

What do you mean by repeated cards. I just skimmed through the code and looks like it may shuffle same card multiple times, but it shouldn't create any duplicate card.

Your shuffling algorithm is based on rand(), and if modular is too small then you might get same index multiple times.

If you don't want to shuffle same index twice, you can simply use a bool vector to check if that index has already been shuffled.

But your code is also good, as in real life game you may shuffle same index/position couple of times.

void Deck::shuffle()
{
    int max = SUIT_SIZE * RANK_SIZE;
    std::vector<bool> alreadyShuffled(max); // it will create a vector of max size, and all index will be initialized as false.
    for(int i=0; i<max; i++)
    {
        int randNum = rand() % 52;
        if(alreadyShuffled[randNum])
             continue;
        alreadyShuffled[randNum] = true;
        swap(cardDeck[i],cardDeck[randNum]);
        cout<<cardDeck[i]<<"   ";
    }
}

What i meant was the card came out multiple times. same as what you mean.

vector are not allowed for my project =)
I asked my friends regards my problem. He claims that the error exists here

swap(cardDeck[i],cardDeck[randNum]);

i shuffled a randNum (1-52) and for sure it will generate same number, resulting in generating same card. I have no idea on how to do the checking

if vector not allowed then use array!! bool alreadyShuffled[SUIT_SIZE * RANK_SIZE]; initialize it with false;

Last post i checked it for index, but if you want to do it for cards, then change the logic for cards.
and if you want to do it exactly 52 times, then you need to decrement your i just before continue;

Here is a suggestion how you can implement it. I notice that you don't have a card Identifier in your Card class, so I added one and used this to check if this card has already shuffeled. Change your code whatever way you like.

class Card{
        int cardIdentifier; //create it from 0 - 51 or 1-52, give each card an unique identifier.
         ......
        int getCardIdentifier(){return cardIdentifier;}
};
void Deck::shuffle()
{
    int max = SUIT_SIZE * RANK_SIZE;
    bool alreadyShuffled[SUIT_SIZE * RANK_SIZE]; //initialize it with false.

    for(int i=0; i<max; i++)
    {
        int randNum = rand() % 52;

       if(alreadyShuffled[cardDeck[randNum].getCardIdentifier()] == false){
             i--; // bad idea as it will shuffle all the cards. dont use it
             continue;
       }
        alreadyShuffled[cardDeck[randNum].getCardIdentifier()] = true;
        swap(cardDeck[i],cardDeck[randNum]);
        cout<<cardDeck[i]<<"   ";
}

Did it help? I got a feeling that you didnt get me. Let me know if I wasn't clear enough. I made couple of changes on the last post's code.

i didnt get it =)

Why don't you make your printDeck() function and not display the cards within the shuffle function? This way you will always print out each card once.

Member Avatar for iamthwee

i shuffled a randNum (1-52) and for sure it will generate same number, resulting in generating same card. I have no idea on how to do the checking

If it is generating the same number, obviously this is where the problem lies.
The swap idea is good. Go with this one.

You're basically picking two elements (1-52) and swapping them. Do this a bunch of times and you have a shuffled deck. Pretty straightforward.

Now the problem is you're creating the SAME random number each time, which suggests you're not seeding the random number. So your next task is to figure out how to seed a random number properly.

I don't get what you all mean =) Hmm btw i tried this.

void Deck::shuffle()
{
    int max = SUIT_SIZE * RANK_SIZE;
    for(int i=0; i<max; i++)
    {
        int randNum = rand() % 52;
        swap(cardDeck[i],cardDeck[randNum]);
        //cout<<cardDeck[i]<<"   ";
    }
    for(int i=0; i<max; i++)
    {
        if(i %13 == 0)
        cout<< '\n' <<'\n';
        cout<< cardDeck[i] << " ";
    }
}
Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.