-1

The code giving the problem is below. I have commented which parts give the correct output, and which not, but I do not know why, and how I should be accessing the class member data normally. I read that trying to access in a C style loop as I am isn't great, but I tried ith iterators only to be met with "error: no match for 'operator<=' (operand types are 'Player' and '__gnu_cxx::__normal_iterator >')"

class Player
{
public:
    Player() {}
    Player(double strength) {}
    virtual ~Player() {}
    double GetStrength() const {
        return Strength;
    }
    void SetStrength(double val){
        Strength = val;
    }
    int GetRanking() const{
        return Ranking;
    }
    void SetRanking(int val){
        Ranking = val;
    }
    int GetATPPoints() const{
        return ATPPoints;
    }
    void SetATPPoints(int val){
        ATPPoints = val;
    }
protected:
private:
    double Strength; //!< Member variable "Strength"
    int Ranking; //!< Member variable "Ranking"
    int ATPPoints; //!< Member variable "ATPPoints"
};

int main()
{
    vector<Player> Players(1000);
    // Player strengths are generated from a normal distribution with a mean of 5 and standard deviation of 2, with values outside the range of 0-10 discarded.
    GeneratePlayerStrengths(Players);

    sort(Players.begin(), Players.end(), SortMethod); // sort descending order

    int i=1;
    for (Player a : Players)
    {   // initialise the rank of the players according to their strength
        a.SetRanking(i);
        i++;
        cout << a.GetRanking() << endl; // returns correctly
    }

    // returns all zeros, then a random number, then more zeros
    for(int j=0;j<32;j++) {
       cout << Players[i].GetRanking() << endl;
    }


    cin.ignore(); // keep the command window open for debugging purposes.
    return 0;

}
Tobias
  • 5,038
  • 1
  • 18
  • 39
whitebloodcell
  • 308
  • 1
  • 4
  • 10
  • 1
    Looks like your problem lies here: `sort(Players.begin(), Players.end(), SortMethod);`. Also iterators can be used the same way as a pointer to the class contained, and dereferenced using the `->` and `*` operators. – πάντα ῥεῖ Jun 10 '14 at 15:42

4 Answers4

5
for (Player a : Players)

In order to modify the objects in your vector through a, it needs to be a reference.

for (Player& a : Players)

Otherwise, you are working with copies which are local to the loop body, and the changes will not be seen when you iterate the next time.

Also, you are using the wrong index variable (i, when you should be using j) in your second loop.

Benjamin Lindley
  • 101,917
  • 9
  • 204
  • 274
  • I don't think this is the issue OP is describing. It's one of many in the code he presented. – luk32 Jun 10 '14 at 15:45
  • 1
    @luk32: Well I think it is the issue the OP is describing. The OP wonders why he is seeing uninitialized values when he iterates through his vector the second time. And the reason is because he never initialized the values in his first loop. – Benjamin Lindley Jun 10 '14 at 15:48
  • @BenjaminLindley This is undoubtedly a problem, but also look at the variable he's using to index into `Players` after the `for`. He's got out of bounds access going on there. And there's a random semi-colon in the middle of that statement, this code probably doesn't even compile. Also, `for(auto& a : Players)` – Praetorian Jun 10 '14 at 15:50
  • My bad. I focused on the error he gave, which was irrelevant actually. The great habit of writing important stuff as comments in code, and irrelevant as the question body caught me. Sorrey. – luk32 Jun 10 '14 at 15:51
  • @Praetorian: Good call. I didn't notice that. – Benjamin Lindley Jun 10 '14 at 15:53
  • Thank you, that explains it and fixed it. Apologies for the other problems/typos in the code, it was compiling, but I then tried many variations and methods to get it to work, so at the point I gave up and asked here it was a mess, which I tried to clean up a bit to simplify the question. The random error I included was primarily because it wouldn't let me post my question without increasing the writing to code ratio. – whitebloodcell Jun 10 '14 at 16:12
4

In the final for-cycle you iterate j variable but use i for the access.

Praetorian
  • 106,671
  • 19
  • 240
  • 328
DOFHandler
  • 225
  • 2
  • 12
2

After fixing the error that Benjamin pointed out, where you print, you should be printing

Players[j].GetRanking() 

instead of

Players[i].GetRanking()
MIke
  • 140
  • 10
0

Considering this code:

for (Player a : Players)
{   // initialise the rank of the players according to their strength
    a.SetRanking(i);
    i++;
    cout << a.GetRanking() << endl; // returns correctly
}

at every loop iteration, you are making a copy of the current Player instance in the vector.

So, you are calling SetRanking() on a copy, not on the original instance of Player stored in the vector.

To fix that, you may want to consider using references:

for (Player& a : Players) // <-- note the & for reference
{
    ... same code ...
}

In this case, a is a reference to the original instance of Player stored in the vector (so, in this case you are not making copies).

In addition, you may want to read this question and answer here on StackOverflow.

Community
  • 1
  • 1
Mr.C64
  • 41,637
  • 14
  • 86
  • 162