0

I have a set of strings and want to add random elements of that set to a list (e.g. distribute poker cards to different players)

I've tried the following:

std::set<std::string> remaining_cards;
std::vector<std::set<std::string>> player_cards;

int random_number;
for (int i = 0; i < number_of_players; ++i) 
{
    random_number = 2;  // for simplicity let's assume the random number is always 2
    auto it = remaining_cards.cbegin();
    std::advance(it, random_number);
    player_cards.emplace_back(remaining_cards.cbegin(), it);  // get one element
    remaining_cards.erase(it);  // remove distributed card from deck
}

Why do I end up with the same card for all players even though I remove the one that got distributed in the last line from the deck with erase?

Nickpick
  • 6,163
  • 16
  • 65
  • 116
  • 2
    By `remaining_cards.erase(it)` you remove only one card from set. In `player_cards.emplace_back(remaining_cards.cbegin(), it); ` you create `set` by invoking constructor taking two iterators - they define range from `remaining_cards` set. So your players must have duplicated cards, is it really what you want to do ? – rafix07 Nov 10 '19 at 18:08
  • iow with emplace_back you are adding multiple cards to a player's hand, but with erase you remove only one card from the deck. – AndersK Nov 10 '19 at 19:49
  • I see, so `emplace_back` takes two iterators. What if I want to add only a single element? – Nickpick Nov 10 '19 at 22:35

1 Answers1

1

I'm not sure why you are using std::set... probably because it sorts the cards automatically. I would use std::vector and sort manually (std::sort).

I have to fill in some blanks on what you are trying to do in the code, as you didn't post a working, complete example.

I would suggest using encapsulation and moving the drawn cards instead of copying before deletion. E.g.

#include <random>
#include <string>
#include <set>
#include <vector>
#include <numeric>

class Random {
private:
    std::default_random_engine generator{};
public:
    int operator()(int maxValue) { return generator() % maxValue; }
};

class Card {
private:
    std::string name{};
public:
    Card() : name{} {}
    Card& operator= (int i) {
        name = std::to_string(i);
        return *this;
    }
    friend bool operator<(Card const& lhs, Card const& rhs) {
        return lhs.name < rhs.name; // or some other sorting.
    }
};

class Player {
private: 
    std::set<Card> cards{};
public:
    void AddCard(Card&& card) noexcept {
        cards.emplace(std::move(card));
    }
};

int main() {
    //fill the deck
    std::vector<Card> deck(42); // instead of remaining cards... why would this be a set?
    std::iota(std::begin(deck), std::end(deck), 1); // fill 1 to 42

    Random random{};

    std::vector<Player> players(4);
    while (deck.size() > 0) { // distribute the whole deck.
        for (auto& player : players) {
            if (deck.empty()) break; // if cards in desck is not equaly dividable between players
            auto randIdx = random(deck.size());
            player.AddCard(std::move(deck[randIdx]));  // move one card
            deck.erase(std::next(std::begin(deck), randIdx));  // and remove it from deck
        }
    }
}
JHBonarius
  • 10,824
  • 3
  • 22
  • 41
  • I see that moving it makes sense with the information I provided. However, in a monte carlo simulation that helps to estimate the probability of winning with given cards, I would then have to recreate the deck again (as I would want to simulate the process many times over). I'm wondering if there's a quicker way to randomly distribute cards from a deck to a vector of sets many times over, without having to recreate the original deck. Maybe pointers could help by pointing onto random cards? But not clear how I can avoid duplicates in that case. – Nickpick Nov 10 '19 at 22:36
  • @Nickpick moving is actually more efficient then copying. But you're asking a new question that's too big for the comments. I do some competitive programming on Codingame and there I indeed tend to use pointers to the cards in play. If you're doing monte carlo, then watch out with things like `std::set`. Because indeed they store the elements by sorting, which is quite slow. You need performance, so use the simplest containers. – JHBonarius Nov 11 '19 at 09:01