1

I'm working on a cards game.

In this game I have players, who are members of a team (2 players per team), and they can join a lobby.

So the lobby has a teams member, and the teams both have a players member. I'd like to iterate over the players directly without going through the teams every time.

Relevant C++ code:

lobby.h

class Lobby {

public:
    Lobby(std::string const& name, std::vector<Team> const& teams = std::vector<Team>());
    Lobby(){};
    ~Lobby();

    // give cards to all players
    void deal();

    std::vector<std::shared_ptr<Player> > getPlayers(); // this is the one I'm trying to implement

   [...]

private:
    std::string _name;
    std::vector<Team> _teams;
};

lobby.cpp

[...]
void Lobby::deal()
{
    vector<Card> cards = Card::getAllCardsShuffled();

    for (int i = 0; i < cards.size(); i++) {
        getPlayers()[i % 4].addCard(cards[i]); // this is how I'd like to use getPlayers()
    }
    cout << _teams[0].getPlayers().at(0).getCards().size() << endl;
    // this compiles but I'm not getting any cards in my players (prints 0)
    // I think a copy is made somewhere, so I'm always getting new players
}


// this is the method I'm trying to implement... 
vector<shared_ptr<Player> > Lobby::getPlayers()
{
    return {
        make_shared<Player>(_teams[0].getPlayers().at(0)),
        make_shared<Player>(_teams[0].getPlayers().at(1)),
        make_shared<Player>(_teams[1].getPlayers().at(0)),
        make_shared<Player>(_teams[1].getPlayers().at(1))
    };
}
[...]

team.h

class Team {
public:
    Team();
    Team(std::string name);
    ~Team();
    void addPlayer(Player const& player);
    void addTrick(Trick const& trick);
    void setScore(int const& newScore);
    void addPoints(int const& score);
    int getScore();
    std::vector<Trick> getTricks();

    std::vector<Player>& getPlayers();
    bool isTeamDealing();

private:
    std::string _name;
    std::shared_ptr<std::vector<Player> > _players;
    int _score;
    std::vector<Trick> _wonTricks;
};

team.cpp

Team::Team(string name)
    : _name(name)
    , _score(0)
    , _players(vector<Player>())
{
}

void Team::addPlayer(Player const& player)
{
    if (_players.size() < 2) {
        _players.push_back(player);
    }
}

vector<Player>& Team::getPlayers()
{
    return _players;
}
[...]

Now the code works when iterating over teams and then players:

void Lobby::deal()
{
    vector<Card> cards = Card::getAllCardsShuffled();

    for (int i = 0; i < cards.size(); i += 4) {
        _teams[0].getPlayers().at(0).addCard(cards[i]);
        _teams[0].getPlayers().at(1).addCard(cards[i + 1]);
        _teams[1].getPlayers().at(0).addCard(cards[i + 2]);
        _teams[1].getPlayers().at(1).addCard(cards[i + 3]);
    }
    cout << getPlayers().at(0)->getCards().size() << endl;
}

... but I'd like to be able to iterate over the players directly, without using 2 nested loops. Is there a simple way to do this?

TL;DR Is there a way to implement getPlayers() that would allow me to iterate over a vector of players and mutate my players directly?

EDIT: I updated the code in Team to have Team.getPlayers() return a reference to the vector instead of a pointer.

Corentin S.
  • 5,750
  • 2
  • 17
  • 21
  • Instead of having Lobby get the players directly, how about having the lobby create the cards and then pass each team their share of cards(by iterator), and then have the Team distribute the cards to its Players? – Nard Oct 19 '14 at 13:03
  • 1
    What is the purpose of all the sharer pointers? Shouldn't the lifetime of a player be well defined? – juanchopanza Oct 19 '14 at 13:08
  • I just realized per 40two's answer that I could use references instead of shared_ptr in Team. So now it looks like this `std::vector& getPlayers();` – Corentin S. Oct 19 '14 at 13:16
  • @Nard I'd also like to be able to use this `getPlayers` method in other parts of the code. I guess I could do what you suggest, but it seems more complicated to me (I'd have to add a method to Team to deal the cards again) – Corentin S. Oct 19 '14 at 13:31
  • 1
    @CorentinS. If you insist on doing so, what I see happening right now is that `make_shared(_teams[0].getPlayers()->at(0))` creates a new `shared_ptr`, creates a new `Player` and assigns the new `shared_ptr` to it, then it copies `_teams[0].getPlayers()->at(0)` by value into this new `Player`. Therefore, whatever changes you perform on this new `Player` will not affect `_teams[0].getPlayers()->at(0)`. To solve this, you simply return a `std::vector` where the reference is directly to `_teams[0].getPlayers()->at(0)`. – Nard Oct 19 '14 at 13:58
  • I'm having trouble defining a `std::vector`, I thought vectors of references where forbidden? Is there a way to make the new pointers in `getPlayers` point to the elements of the original vectors (without calling the constructors again?) – Corentin S. Oct 19 '14 at 14:03
  • See http://stackoverflow.com/a/23488449/1418983 – Corentin S. Oct 19 '14 at 14:04

1 Answers1

2

So I found a way that gives me what I wanted, using std::reference_wrapper:

vector<reference_wrapper<Player> > Lobby::getPlayers()
{
    return {
        _teams[0].getPlayers().at(0),
        _teams[0].getPlayers().at(1),
        _teams[1].getPlayers().at(0),
        _teams[1].getPlayers().at(1)
    };
}

void Lobby::deal()
{
    vector<Card> cards = Card::getAllCardsShuffled();

    for (int i = 0; i < cards.size(); i++) {
        getPlayers().at(i % 4).get().addCard(cards[i]);
    }
}

If anyone sees a better way, I'm all ears though.

Corentin S.
  • 5,750
  • 2
  • 17
  • 21