Thread: class member functions gone haywire.

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.