2

After getting help on this question, I was led to do better debugging. In that process, I discovered that my problem is this:

While working in C++, attempting to set a class' member variable to a value works, but not while looping. I have reduced my code (to follow) to what I believe is the simplest for that still produces the error.

Calling a function of the class Mover that modifies the variable pMovXPOS, which can then be retrieved as updated within the same scope (within that function) and from the where it was called (within the loop). However, upon looping, it seems the variable is reset to its original value.

I'm posting the entirety of the test code here. The problem lies in the RunWorld() function of the Main-test.cpp file. If you compile and run, you should see the output that shows the variable changing, then being reset.

Is this a scope issue? A construction/destruction issue? A pointer/reference issue? I'm not sure where to begin (beyond better debugging).

(As I am new to C++, I'm sure there are some glaring issues with style and/or methods I've used. If there are any major no-nos, please free to point those out.)

Thanks in advance for any help!

//Main-Test.cpp
#include "Globals-Test.h"
#include "Mover-Test.h"

std::vector < Mover > AllMovers;

long SysCounter;

Mover CreateNewMover() {
Mover TempMover;

    TempMover.setXPOS(5);
    TempMover.setYPOS(10);

    return TempMover;

}

void RunWorld() {
Mover TempMover;
unsigned int iMoverLoop;

    srand ( time(NULL) );

    AllMovers.push_back(CreateNewMover());

    for (SysCounter = 0; SysCounter <= 50; SysCounter++) {
        for (iMoverLoop = 0; iMoverLoop < AllMovers.size(); iMoverLoop++) {
            std::cout << "Loop #:" << SysCounter << std::endl;
            TempMover = AllMovers.at(iMoverLoop);
            std::cout << "Is: " << TempMover.getXPOS() << std::endl;
            TempMover.DoMove();             
            std::cout << "Is: " << TempMover.getXPOS() << std::endl;
        }
    }
}



int main() {
    RunWorld();
    return 0;
}

//Globals-Test.h
#include <stdlib.h>
#include <stdio.h>
#include <iostream>
#include <sstream>
#include <unistd.h>
#include <ctype.h>
#include <math.h>
#include <string>
#include <vector>
#include <time.h>
#include <fstream>

//Mover-Test.h
extern long MoverIndex;

class Mover {

private:

    int pMovXPOS;
    int pMovYPOS;

public:

    int getXPOS();
    void setXPOS(int newXPOS);
    int getYPOS();
    void setYPOS(int newYPOS);

    Mover();
    ~Mover();

    void DoMove();

};

//Mover-Test.cpp
#include "Globals-Test.h"
#include "Mover-Test.h"

Mover::Mover() {    

}

Mover::~Mover() {

}

int Mover::getXPOS() {
    return pMovXPOS;
}

void Mover::setXPOS(int newXPOS) {
    pMovXPOS = newXPOS;
}

int Mover::getYPOS() {
    return pMovYPOS;
}

void Mover::setYPOS(int newYPOS) {
    pMovYPOS = newYPOS;
}

void Mover::DoMove() {
pMovXPOS = pMovXPOS + 1;
pMovYPOS = pMovYPOS + 1;

}

//Compiled with:
g++ -Wall -lm -c Main-Test.cpp
g++ -Wall -lm -c Mover-Test.cpp
g++ -Wall Mover-Test.o Main-Test.o -o world-test.exe -lm
Community
  • 1
  • 1
Gaffi
  • 4,307
  • 8
  • 43
  • 73
  • 6
    Can you make a *minimal* example that shows the problem? – Flexo Feb 22 '12 at 11:06
  • Yes, divide and conquer. Break it down to a minimal example. You might find it works okay. Then you'll be able to add parts back until it stops working and you can see what the problem is. – Peter Wood Feb 22 '12 at 11:14
  • Reduced, please see above. Still doesn't work. :-/ – Gaffi Feb 22 '12 at 11:17
  • 2
    @Gaffi You shouldn't add ANSWERED or similar tags to your title on stack overflow. The list of questions shows you how many people have answered, and the number turns yellow if an answer has been accepted, so there's no need for any other indication. – obmarg Feb 22 '12 at 11:39
  • 1
    @Gaffi By reduce, I mean to about 15 lines of code maximum. – Peter Wood Feb 22 '12 at 11:54

1 Answers1

5

Your problem is this line:

TempMover = AllMovers.at(iMoverLoop);

You're creating a copy of the Mover that is at the index iMoverLoop, then modifying that copy. The object that is in the vector is never modified and on the next iteration your changes are lost, as TempMover is overwritten with the next copy from AllMovers

One way to get around this would be to use a reference for TempMover instead. For example:

Mover& tempMover = AllMovers.at(iMoverLoop);
tempMover.DoMove();
obmarg
  • 9,369
  • 36
  • 59
  • +1, although TempMover doesn't go out of scope in the loop, it is reset with the value of the element in the vector at the start of each loop. – Skizz Feb 22 '12 at 11:23
  • @Skizz Ah yes, cheers for pointing that out, wasn't paying too much attention to the code as I wrote my answer. Fixed now – obmarg Feb 22 '12 at 11:24
  • Was it really so simple!?! I had tried using the reference indicator, but at the onset of the function, not where the variable was assigned, and that made the difference. Thank you so much! – Gaffi Feb 22 '12 at 11:27