class member functions gone haywire.

This one is absolutely amazing. I don't get how this bug is even possible. My program tries to read in class data from a formatted text file. Somehow all my classes data gets changed to the same piece of data in a file. I'll explain more at the bottom

Agent Class: (pay attention to operator>>, Thats where my problem begins.)

Code:

class agent
{
      public:
             agent();
             agent(int idNum, string clntNm, string clntFT, string pTC, agent *next);
             agent(const agent&);
             ~agent(){listLength--;}
             int getIdNum() const {return idNumber;}
             void setIdNum(int newId) {idNumber = newId;}
             string getClientName() const {return clientName;}
             void setClientName(string newClientName) {clientName = newClientName;}
             string getFileType() const {return clientName;}
             void setFileType(string newFileType) {clientFileType = newFileType;}
             string getPath() const {return clientName;}
             void setPath(string newFilePath) {pathToClient = newFilePath;}             
             agent *getNextAgent() const {return nextAgent;}
             void setNextAgent(agent *newNextAgent) {nextAgent = newNextAgent;}
             void setNextAgent(agent &newNextAgent) {nextAgent = &newNextAgent;}
             void display();
             agent operator=(agent&);
             friend ifstream& operator>>(ifstream&,agent&);
             friend ofstream& operator<<(ofstream&,agent*);
             static int listLength; 
      private:
              int idNumber;
              agent *nextAgent; //a pointer to the next agent
              string clientName; //its clients name
              string clientFileType; //and its clients file type
              string pathToClient; //where client is located on Hard Disk

};
int agent::listLength = 0;

agent::agent():
               idNumber(0),
               nextAgent(0)
{
               string str = " ";
               clientName = str;
               clientFileType = str;
               pathToClient = str;
}
//copy constructor
agent::agent(const agent& rhs):
                   idNumber(rhs.getIdNum()),
                   nextAgent(rhs.getNextAgent()),
                   clientName(rhs.getClientName()),
                   clientFileType(rhs.getFileType()),
                   pathToClient(rhs.getPath())
                   {}

agent::agent(int idNum, string clntNm, string clntFT, string pTC, agent *next): idNumber(idNum), 
clientName(clntNm), clientFileType(clntFT), pathToClient(pTC)
{
                    listLength++;
                    nextAgent = next;
}//agent::agent(int,string,string,string,agent*)
void agent::display()
{     
     cout<<idNumber << "\t" <<clientName<< "|\t" << clientFileType << "|\t"<< pathToClient;
     cout<< "..." << endl;
}
agent agent::operator=(agent &agentFull) //..::!untested!::..
{
     clientName = agentFull.getClientName() ;
     idNumber = agentFull.getIdNum();
     clientFileType = agentFull.getFileType();
     pathToClient = agentFull.getPath();
     nextAgent = 0;
     return *this;
}
ifstream& operator>>(ifstream& rfin,agent &rnext)
{
      char ch;
      string s1,s2,s3,sid;
      while (rfin.get(ch)) //read a character
      {
      // cout<<"\'"<<ch<<"\'" <<endl;
       if (ch == 'a') //is it the begining of an agent
       {
        rfin.get(ch); //eat up newline
        getline(rfin,s1, '\n'); 
        getline(rfin,s2, '\n');
        getline(rfin,s3, '\n');
        cerr<<"*!*"<<s1<<"!"<<s2<<"!"<<s3<<"*!*"; //data is fine
        rnext.setIdNum(GLOBALid + 1);    //define agent
        GLOBALid++; //don't want to assign all agents with an id of 1
        rnext.setClientName(s1);  //Problem lies in these
        rnext.setFileType(s2);
        rnext.setPath(s3);  
        rnext.setNextAgent(0);
        cerr<<"!"<<rnext.getPath(); //agent is bad here
        return rfin;                                   
       }//if
      }//while
      return rfin;
}
ofstream& operator<<(ofstream& lhs,agent* rhs)
{
          lhs <<'a' << '\n';   //formatting
          lhs << rhs->getClientName() << '\n';
          lhs << rhs->getFileType() << '\n';  
          lhs << rhs->getPath() << '\n';
          lhs << '\n'; //formatting
          return lhs;
}

Here's where I call operator>>:

Code:

fileManager::fileManager()
{}
//..::FUNCTION file2Agent()::..
//reads a file and creates a vector of agents (not pointers to)
vector <agent>& fileManager::file2Agent(vector<agent> &parray)
{
      agent empty;
      empty.setIdNum(0);
      
      ifstream fin("agents.DATA");
      if(!fin) //did we get the file open?
      {
       cerr<<endl<<endl<<"\aCouldn't open file in vector <agent>& fileManager::file2Agent()"; //nope
       return parray;
      }//if
      
      while (fin >> empty) //get an agent
       {
        cerr<<empty.getPath();
        parray.push_back(empty); //put it into a vector
       }//if
       
      fin.close();
      return parray;      
}//file2Agent

here's the text file it reads from:

Code:

a
this
is
testing1

a
this
is
testing2

a
this
is
testing3

a
this
is
testing4

a
this
is
testing5

a
this
is
testing6

in operator>>:The program reads in the data from the file correctly but after it sets the agents values with the data the agents all get set with the first piece of data read in for that agent.

if I display the list of agents here's what happens:

Code:

1 this| this|this
2 this| this|this
3 this| this|this

or if the file looks like this:

Code:

a
asdf
gfs
gfhtr

a
tye
nf
dfg

a
er
vfd
ew

the output of the list looks like this:

Code:

1 asdf|asdf|asdf
2 tye|tye|tye|
3 er| er| er

some how either the set functions in the agent class have gone crazy or (more likely) the get functions are all returning the same variable. I don't get how this would happen and no matter how many times I go through my code I don't get it. I've spent 3 days on this bug now and I've run out of Ideas.

There are a lot of oddities with this class.
Whilst it is very difficult to pinpoint why what happens is happening, I can virtually guarantee that if you stop doing some of the bazaar things that your doing at the moment, and fix the serious bugs, then it will behave.

1. There is no need to define a copy-constructor when all it does is invoke the copy-constructor on all its members. A default generated copy-constructor deos exactly the same. You shouldn't be using 'get' functions in there either, as a class knows about its own variables anyway.
2. You have two versions of setNextAgent. There is no need for that. Remove the one that takes a reference.
3. Your destructor decrements listLength, but your copy-constructor and default constructor do not increment it. That will cause havoc in a container. Of course if you really do need this count, then you will need to supply your own copy-constructor definition to keep accurate. However it is a very dubious thing for the class itself to know about how many instances of it there are. The conatiner it's in already tracks this.
4. Operator = should return a reference. Again, don't use 'get' function here. I makes no sense to protect a class from itself.
5. Your operator = does not 'copy' the nextAgent parameter, it instead sets the copy to zero. This will not play well with the container. A class such as this probably has no business keeping track of other instances in its own list inside itself either. Use a seperate list outside of the class.

Actually come to think of it, the problem is almost certainly caused by the nextAgent pointer in this class. You cannot keep pointers to items in a vector because its resizing moves those objects.
Remove nextAgent and listLength altogether! (and the get/set functions for it) This class should not be trying to do the job of the container.

Some things to try.
1. Delete the "debug" and "release" directories (unless you also put some of your own data files there for convenience), then do a "rebuild all". Then retest.

2. Create a copy of your project and start removing all the code which is not essential to showing the problem. That is the bare minimum to read the file and print the result. Keep testing as you do this.
One of two things will happen:
- you'll remove some apparently unrelated code and it will start working as expected. Study the part most recently removed, because it's likely that has something to do with the problem.
- you'll reach a minimum which you can post here in it's entirety which we can compile and try for ourselves.

Code:

             void setNextAgent(agent &newNextAgent) {nextAgent = &newNextAgent;}

This looks like begging for code like this:

Code:

void somefunc() 
{
  agent a;
  ... // Code that builds agent a .. 
  setNextAgent(a);
}

This will link in an object that lives on the stack in your function, and will not exist any longer when you leave the function.

Edit:

Code:

1 asdf|asdf|asdf
2 tye|tye|tye|
3 er| er| er

This looks like a typical case of storing the address of a string rather than copying the string somewhere in your code.

--
Mats

Code:

string getClientName() const {return clientName;}
string getFileType() const {return clientName;}
string getPath() const {return clientName;}

Is this intended? Your use of these functions in the operator<< code would cause the same string to be output.

Code:

ofstream& operator<<(ofstream& lhs,agent* rhs)
{
          lhs <<'a' << '\n';   //formatting
          lhs << rhs->getClientName() << '\n';
          lhs << rhs->getFileType() << '\n';  
          lhs << rhs->getPath() << '\n';
          lhs << '\n'; //formatting
          return lhs;
}

This is typically implemented (again) using references, since the object being output is not (typically) modified by the output call, it is passed as a const reference. Further, since the function is a friend of the agent class, it has access to agent's private data members and does not need the use of the agent classes get???? member functions:

Code:

ofstream& operator<<(ofstream& lhs,const agent& rhs)
{
          lhs <<'a' << '\n';   //formatting
          lhs << rhs.clientName << '\n';
          lhs << rhs.clientFileType << '\n';  
          lhs << rhs.pathToClient << '\n';
          lhs << '\n'; //formatting
          return lhs;
}

1. There is no need to define a copy-constructor when all it does is invoke the copy-constructor on all its members. A default generated copy-constructor deos exactly the same. You shouldn't be using 'get' functions in there either, as a class knows about its own variables anyway.

That does seem strange. Removed. Now display outputs the correct values (it access data directly, this means removing the get functions from this program should fix this but lets go down the list)

2. You have two versions of setNextAgent. There is no need for that. Remove the one that takes a reference.

these two are related

Code:

             void setNextAgent(agent &newNextAgent) {nextAgent = &newNextAgent;}

This looks like begging for code like this:

Code:

void somefunc() 
{
  agent a;
  ... // Code that builds agent a .. 
  setNextAgent(a);
}

This will link in an object that lives on the stack in your function, and will not exist any longer when you leave the function.

Actually I realized the potential problems that this could cause when I wrote that. I wrote the second setNextAgent to work around some other code that looks like this:

Code:

void someFunct()
{
 agent a;
 someOtherFunct(a,agent*);
}
void someOtherFunct(agent& a, agent*pa)
{
 //build agent code
}

3. Your destructor decrements listLength, but your copy-constructor and default constructor do not increment it. That will cause havoc in a container. Of course if you really do need this count, then you will need to supply your own copy-constructor definition to keep accurate. However it is a very dubious thing for the class itself to know about how many instances of it there are. The conatiner it's in already tracks this.

That was from an earlier design of this class. I forgot to remove it. I never use it in any code. Removed.

4. Operator = should return a reference. Again, don't use 'get' function here. I makes no sense to protect a class from itself.

I don't even need that function. I deleted it.

5. Your operator = does not 'copy' the nextAgent parameter, it instead sets the copy to zero. This will not play well with the container. A class such as this probably has no business keeping track of other instances in its own list inside itself either. Use a seperate list outside of the class.

The not copying the next agent parameter was intentional. operator= is gone now though.
I've been planning to remove the agent class from the process of keeping track of itself. I think I'll do that on Friday.

Actually come to think of it, the problem is almost certainly caused by the nextAgent pointer in this class. You cannot keep pointers to items in a vector because its resizing moves those objects.

The vector doesn't always keep track of the agents. The nextAgent pointers are all set to 0 before entering the vector and are refilled once they leave the control of the vector. I don't use the vectors to store them all the time because I like the flexibility of switching to other containers periodically that can accomplish certain tasks better. The agents keep track of themselves like an intermediate container before switching. This seems strange to me reading over my last two sentences. Maybe I should rework this.

1. Delete the "debug" and "release" directories (unless you also put some of your own data files there for convenience), then do a "rebuild all". Then retest.

Will try in a moment.

Create a copy of your project and start removing all the code which is not essential to showing the problem. That is the bare minimum to read the file and print the result. Keep testing as you do this.
One of two things will happen:
- you'll remove some apparently unrelated code and it will start working as expected. Study the part most recently removed, because it's likely that has something to do with the problem.
- you'll reach a minimum which you can post here in it's entirety which we can compile and try for ourselves.

That is a very good debugging strategy. My next post in this topic(and posts from now on) will be the results of that. :-)

Code:

string getClientName() const {return clientName;}
string getFileType() const {return clientName;}
string getPath() const {return clientName;}

Is this intended? Your use of these functions in the operator<< code would cause the same string to be output.

I hate myself... That was the problem. Thank you. I've checked the get functions millions of times and never saw that. Maybe the fireplace is leaking carbon monoxide. That'd give me an excuse for my pure lack of attention. Such a stupid mistake too. I don't know how they stayed like that for so long. I must not be using the get functions.

Code:

ofstream& operator<<(ofstream& lhs,agent* rhs)
{
          lhs <<'a' << '\n';   //formatting
          lhs << rhs->getClientName() << '\n';
          lhs << rhs->getFileType() << '\n';  
          lhs << rhs->getPath() << '\n';
          lhs << '\n'; //formatting
          return lhs;
}

This is typically implemented (again) using references, since the object being output is not (typically) modified by the output call, it is passed as a const reference. Further, since the function is a friend of the agent class, it has access to agent's private data members and does not need the use of the agent classes get???? member functions:

That originally used a reference. I think I changed it to a pointer in desperation. I forgot about how friend functions worked. Maybe I should get more acquainted with them. I'll go read a tutorial on them to refresh my memory.

Thanks for all of your(plural your?) help.

The post icons need an animated 2 frame smiley hitting a board on his head...

Please fix your quotes. You use code tags instead of quote tags, thus ruining the layout.

Originally Posted by A10

The post icons need an animated 2 frame smiley hitting a board on his head...

Here you go, I used these in another recent post, they may be close to what you wanted:

Originally Posted by CornedBee

Please fix your quotes. You use code tags instead of quote tags, thus ruining the layout.

My posts in this thread don't have edit buttons

Here you go, I used these in another recent post, they may be close to what you wanted:

that left one is perfect

EDIT: Only this one does. Do the edit buttons have a time limit?

Possible, though I've seen people edit the code in their original post when the threads were pretty old.

Anyway, I fixed it for you.