0

i'm a student and very new to C++, i figured i wanted to try the notions i recently learned to make a little card game.

I built a Class card that stores the value and the suit, here is the constructor :

Card::Card(unsigned int value, unsigned int color, unsigned int id)
:
_value(value),
_color(color),
_id(id)
{
}

Then i made a Deck class (.hpp below) :

class Deck
{
    public:
        Deck();
        ~Deck();
        void add(Card card);
        void remove();
        void randomize();
        void display();
        std::vector<Card> getCards()const;

    private:
        std::vector<Card> cards;

};

Constructor :

Deck::Deck()
{

    int i = 0;
    while (i < 52)
    {
        this->cards.push_back(Card( i%13, i%4, i));
        i++;
    }
}

my problem is with the Function randomize() , when i call it the output remains the same no matter what, my school asks us to compile with -std=c++98 so i first tried the simple std::random_shuffle(cards.begin(),cards.end()) approach but the output seemed to never change.

i tried in C++11 with std::shuffle but have no more success, the decks stays ordered.

here is my randomize function :

void Deck::randomize()
{
    std::random_device rd;
    std::default_random_engine rng(rd());

    std::shuffle(this->cards.begin(), this->cards.end(), rng);
}

This is my first post on stack overflow, please tell me if i am not formatting questions properly, i will change it in the future.

Malga
  • 9
  • 1
  • probably shouldn't name members with underscore in front, those are usually reserved for implementation – Asphodel Feb 10 '22 at 22:04
  • 1
    `std::vector getCards()const;` -- This will return a coy of the `Card` vector, and not the actual `Card` vector that is stored in the object. Are you calling this, and expecting that the vector is the same one in the `Deck` object? If you are, then that may explain the issue. You should be returning a reference. – PaulMcKenzie Feb 10 '22 at 22:07
  • 2
    That call looks correct. Are you *certain* the context in which you're invoking that specific instance of `Deck`'s randomize method is not itself being passed a `Deck` by value and thus discarding the modified deck when ultimately returning back to the caller ? (fyi, that was a not-so-subtle hint to stitch up a proper [mcve]). – WhozCraig Feb 10 '22 at 22:09
  • @Asphodel member variables beginning with an underscore, followed by a lower-case letter [are fair to use](https://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier). – Drew Dormann Feb 10 '22 at 22:11
  • @Asphodel Those are only reserved [at global namespace](http://eel.is/c++draft/lex.name#3.2) (but if `_` is followed by a capital letter, it's reserved everywhere). – HolyBlackCat Feb 10 '22 at 22:13
  • @PaulMcKenzie Hello, thank you for your answer, the getCards function is used by my overload << iterator and the Deck.display() fuction, i supposed that when they are called after a call to deck.randomize() the copy should be randomized – Malga Feb 10 '22 at 22:14
  • 2
    @Malga a [mre] may be necessary to discuss the behavior of this code. – Drew Dormann Feb 10 '22 at 22:16
  • @Malga -- I pointed this out in case you expected the same card vector is returned. If you were to use `getCards` in a place where it does matter (which can easily be done), then you would either have weird behavior, or undefined behavior. – PaulMcKenzie Feb 10 '22 at 22:16
  • 1
    You actually did a fine job of formatting your question, as well as making your problem quite clear. Kudos. The one obvious improvement would be to include enough more code to have a complete program we can compile that demonstrates the problem. – Jerry Coffin Feb 10 '22 at 22:17
  • As was mentioned above. Show us more – Taekahn Feb 10 '22 at 22:19
  • Unrelated: `std::random_device rd;` and `std::default_random_engine rng(rd());` are pretty expensive. You probably don't want to (or need to) do this every time you shuffle a deck. Setting up the RNG once at the beginning of the program is usually enough. – user4581301 Feb 10 '22 at 22:24
  • i read your comments about the reproductible example , is it ok to post a link to a git with a makefile or should i just post code? – Malga Feb 10 '22 at 22:27
  • I pretty much copied your code. Only thing I can figure is you didn't call `Deck::randomize`. It is shuffled [here](https://godbolt.org/z/EsPWMoWW7) – lakeweb Feb 10 '22 at 22:54
  • @Malga part of pruning your code down to a minimal example is that if you do so, you often solve your problem – Taekahn Feb 10 '22 at 23:06
  • thanks lakeweb , your code works on my machine i really don't get it now x) i tried to change my ostream << operator like you did but more likely it is a scope issue because i changed my main code to use yours and it didn'r work, maybe i should try instianciate with New in my deck constructor and delete in the destructor Edit: (also working right now on making a smaller example right now, sorry i'm a bit slow still :p) – Malga Feb 10 '22 at 23:16
  • You are welcome. But don't reach for `new,delete`, your code is sound with what you have. And, use your debugger! Stepping through and looking at what happens is so useful. – lakeweb Feb 10 '22 at 23:21
  • i don't know what to think ... while making my minimal example now it looks like my code works fine ... at least i have a base to work on now ! – Malga Feb 10 '22 at 23:35

0 Answers0