c++ runtime error with random_shuffle of vector in a class

I am trying to code a game of Blackjack. I have been self-learning C++ in my spare time, and this is my first time posting on any website about programming.

I have been looking up the answers to my problems as I go along, and have learned quite a lot.. but this problem has completely baffled me. I fear that I am approaching the task completely wrong, and hopefully you can help me with this.

I have a Card class, and a Deck class that holds a vector of 52 Cards. The vector is a private member of Deck class, and I fear this is my problem?

When I add the random_shuffle line to my code, it compiles fine but then the console window crashes (Windows 7 x64, code::blocks, c++). I can't work out what I am doing wrong. I call the vector random access iterators begin() and end()...

deck.h

#ifndef DECK_H
#define DECK_H

#include <vector>

using namespace std;

/** Card Class */
class Card
{
public:
/** Constructor prototypes */
//Card(); //default constructor
Card(int s, int r) : suit(s), rank(r) {} 

/** GET function prototypes */
int getRank(); // returns card number as int
string getSuit(); // returns the suit in a string

private:
int rank;
int suit;
} ;

/** Deck class */
class Deck
{
public:
Deck();
vector <Card> get_deck() { return deck; };

private:
vector<Card> deck;
};

#endif // DECK_H

deck.cpp

#include <iostream>
#include <string>
#include <vector>
#include "deck.h"

using namespace std;

/** Deck ctor to initialise deck */
Deck::Deck()
{
for(int suit = 0; suit < 4; suit++)
{
    for(int rank = 0; rank < 13; rank++)
    {
        deck.push_back(Card(suit,rank));
    }
}

}

/** Functions to GET rank and suit */
// Function to get rank as int
int Card::getRank()
{
return rank;
}

// Function to get suit as string
string Card::getSuit()
{
switch(suit)
{
    case 0:
    return "Diamonds";

    case 1:
    return "Hearts";

    case 2:
    return "Clubs";

    case 3:
    return "Spades";

    default:
    return "Error";
}
}

main.cpp

#include <iostream>
#include <algorithm>
#include <ctime> // time()
#include <string>
#include <vector>

#include "deck.h"

using namespace std;

int main()
{

Deck mydeck;

random_shuffle( mydeck.get_deck().begin(), mydeck.get_deck().end() );

// Loop to iterate through deck of cards
for(int i = 0; i<52; i++)
{
    cout << mydeck.get_deck()[i].getRank() << " of " << mydeck.get_deck()[i].getSuit() << endl;
}

// Display size of deck
//cout << endl << "The size of deck is: " << mydeck.get_deck().size() << endl;


return 0;
}

Any help or words of wisdom will be much appreciated, and I hope I formatted everything right...

Many Thanks

Dan

Solution

This accessor method:

vector <Card> get_deck() { return deck; };

Returns a copy of the vector of cards. So when you call it twice, you get two different copies, and the begin() of the first copy doesn't match up with the end() of the second copy, so it crashes.

To fix it, you should return the array by reference so that a copy is not made:

vector <Card>& get_deck() { return deck; }  // no semicolon needed here
//           ^
//           |
//    this is a reference

However, this allows the caller to modify the internal array, which is generally a bad idea. To avoid this, you should return it by const reference:

const vector <Card>& get_deck() { return deck; }

But if you do that, then std::random_shuffle can't modify the array. So to fix that, the ideal solution would be to add a class method to the Deck class which calls random_shuffle on itself.