C++ Object doesn't save value

Question

Objects I created for the Player class does not change the private variable even tho it does in the function that changes it. There maybe some reference problems with the objects being used in the main() function but I can't figure out where or why.

Specifically the loop at the end of the main file prints the games won by the players in the vector Players but all the scores return 0. While when the player object.matchWon() function correctly parse and print the score and shows the change made to the private variable.

I suspect there are problems with the usage of getPlayer() function in the main code.

Thanks in advance!!

Player.cpp:

#include "Player.h"
#include <stdlib.h>
#include <iostream>

using namespace std;
//constructor
Player::Player(string first, string last) {
    Player::first = first;
    Player::last = last;
    gWins = 0;
    gLoss = 0;
    mWins = 10;
    mLoss = 2;
}

//setters
void Player::addLoss(int increment) {
    this->gLoss+=increment;
}

void Player::addWins(int increment) {
    this->gWins+=increment;
}

void Player::matchWon(vector<string> scores) {
    for (int i = 5;i<scores.size()-1;i++){
        cout<<(int)scores[i][0]-'0'<<"-";
        cout<<(int)scores[i][2]-'0'<<endl;
        gWins+=scores[i][0]-'0'; //add games won
        cout<<gWins<<endl;
        gLoss+=(int)scores[i][2]-'0';   //add games lost
    }
    this->mWins++;
}

void Player::matchLost(vector<string> scores) {
    this->mLoss++;
}

double Player::winPercentage() {
    return (double)mWins / mLoss;
}

//accessors
string Player::getFirstname() {
    return this->first;
}

string Player::getLastname() {
    return this->last;
}

int Player::getGameswon() {
    //cout<<gWins;
    return this->gWins;
}

main.cpp:

#include <iostream>
#include "player.h"
#include <fstream>
#include <vector>
#include <sstream>

using namespace std;

vector<string> split(const string &s) {
    vector<string> elems;
    istringstream iss(s);
    do
    {
        string sub;
        iss >> sub;
        elems.push_back(sub);

        //cout << "Substring: " << sub << endl;

    } while (iss);

    return elems;
}

Player &getPlayer(vector<Player> &players, const string &first, const string &last){
    for (int i=0;i<players.size();i++){
        if (players[i].getFirstname()==first&&players[i].getLastname()==last){
            return players[i];
        }
    }
    players.push_back(Player(first,last));
    return (players[players.size()-1]);
}

int main(int argc, char *argv[]) {

    ifstream file(argv[1]);
    ofstream ofile(argv[2]);
    if (!file.is_open()){
        cerr <<"Could not open file\n";
        return 0;
    }

    string line;
    vector<Player> players;

    while (getline(file,line).good()){

        vector<string> lineParsed = split(line);
        vector<string> matchData = split(line);

        Player p1 = getPlayer(players,lineParsed[0],lineParsed[1]);
        Player p2 = getPlayer(players,lineParsed[3],lineParsed[4]);

        p1.matchWon(lineParsed);
        cout<<p1.getFirstname()<<"!"<<p1.getGameswon()<<endl;
    }

    for (int i=0;i<players.size();i++){
        //cout<<players.size();
        cout<<players[i].getFirstname()<<":"<<players[i].getGameswon()<<endl;
    }


    return 0;
}

look this answer http://stackoverflow.com/questions/19608535/passing-a-vector-by-reference-to-function-but-change-doesnt-persist — Shvet Chakra, Feb 09 '16 at 05:34

alcedine · Accepted Answer · 2016-02-09T05:39:40.643

3

You're copying your players:

Player p1 = getPlayer(players,lineParsed[0],lineParsed[1]);
Player p2 = getPlayer(players,lineParsed[3],lineParsed[4]);

so when you p1.matchWon(), it happens to the local Player object, not the one in the vector.

Although the return type of getplayer() is Player&, that doesn't matter if you're assigning it to a non-reference variable. If you want to be modifying the Player instances in the vector, you should have

Player& p1 = getPlayer(players,lineParsed[0],lineParsed[1]);
Player& p2 = getPlayer(players,lineParsed[3],lineParsed[4]);

By the way, getPlayer() is unsafe. The vector might resize itself when you push_back to it, invalidating references. You may want to store a vector of pointers instead (or just make sure through e.g. resize() that the vector definitely has room for all the players you want to push to it).

edited Feb 09 '16 at 05:39

answered Feb 09 '16 at 05:33

alcedine

909
5
18

Thank you for the response!! When the vector resizes, it may change the internal references for stored variables? is that what you mean by invlidating references? – youngin123 Feb 09 '16 at 15:06
The contents of the vector may have to move to a different memory location. See [this question](http://stackoverflow.com/questions/8261037/what-happen-to-pointers-when-vectors-need-more-memory-and-realocate-memory) for specifics. – alcedine Feb 09 '16 at 15:12

score 0 · Answer 2 · edited Feb 09 '16 at 05:30

0

It would help if you included the class definition. There seems to be something strange going on:

Player::first = first;
Player::last = last;

Are these static fields? If they are, the issue is that you only store the first and last (names?) of the last player you create.

edited Feb 09 '16 at 05:30

jblixr

1,345
13
34

answered Feb 09 '16 at 05:20

DAAlex

446
5
9

C++ Object doesn't save value

2 Answers2