Class creating multiple objects of another class at the same memory location (C++)

Question

So, I've got this class that contains a vector of another class. Whenever I try to push a new object into this vector, it's creating that object at the same memory location each time.

The (hopefully) relevant code:

class FSM{
    private:
        std::vector<Node> nodeList;
        int cap;
        int obs;
        int topNode;

    public:
        FSM(int nodeCap, int numObs){
            cap = nodeCap;
            obs = numObs;
            topNode = -1;
        }

        bool addNode(){
            if (isFull()) return false;
            nodeList.push_back(Node(obs));
            topNode++;
            return true;
        }

Now, if I create a stand-alone Node object in my main function and cout the &node, I get different memory locations. But the ones created in the FSM class are always the same. Also, if I change anything in one of the Nodes stored by the FSM class, it changes it for all of them. I have no idea what's going on.

EDIT: As requested, here is the Node class. Just gonna post the whole thing, not sure what is relevant.

class Node{
    private:
        std::vector<int> connects;
        int action;

    public:
        Node(int numObs){
            for(int i = 0; i < numObs; i++){
                connects.push_back(-1);
            }
            srand(time(NULL));
        }

        void setConnections(std::vector<int> newVec){
            for (int i = 0; i < connects.size(); i++){
                connects[i] = newVec[i];
            }
        }

        int getConnection(int index){
            return connects[index];
        }

        std::vector<int> getConnectionList(){
            return connects;
        }

        void setAction(int act){
            action = act;
        }

        int getAction(){
            return action;
        }

        void setRandomConnections(int numNodes){
            for (int i = 0; i < connects.size(); i++){
                connects[i] = rand() % numNodes;
            }
        }
};

EDIT the Second: Here's what my main is doing.

int main(){
FSM myFSM(5, 3);
while (!myFSM.isFull()){
    myFSM.addNode();
    std::cout << &myFSM.getTopNode(); // getTopNode() returns the most recent
                                              // node.
}
}

So are you pushing multiple nodes to the vector? Or are you just re-running your program several times? — Drew McGowen
– Drew McGowen, Commented Aug 2, 2013 at 19:07
Can we see the code that obtains a Node in order to display its address? I suspect that this code is where the problem lies. — cdhowie
– cdhowie, Commented Aug 2, 2013 at 19:09
If getTopNode does what I think it does, you're printing the address of a temporary object (aka a copy of the top node, not the top node itself). So that code is meaningless. — Borgleader
– Borgleader, Commented Aug 2, 2013 at 19:30
Show code that can be copied, pasted, compiled and run. It isn't anyone's idea of fun to reconstruct all the little missing bits. — n. m. could be an AI
– n. m. could be an AI, Commented Aug 2, 2013 at 19:40

Borgleader · Accepted Answer · 2013-08-02 19:41:18Z

4

If getTopNode does what I think it does, you're printing the address of a temporary object (aka a copy of the top node, not the top node itself). So that code is meaningless.

Here I've implemented a print function for the locations of the nodes in FSM:

void printNodeLocations()
{
    for(Node& n : nodeList) { std::cout << &n << std::endl; }
}

And I get different ones as expected:

0x8ad3018
0x8ad301c

EDIT: I cannot reproduce your claim that changing one node changes all of them. See updated code

edited Aug 2, 2013 at 19:41

answered Aug 2, 2013 at 19:35

Borgleader

15.9k5 gold badges48 silver badges63 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

MRoush Over a year ago

Hmm...Let me check that out. I think I might know the issue with the node changing thing, but I think you're right here.

Borgleader Over a year ago

@MRoush I've added an update code sample where I change one of the node's values. The other node(s) are not affected.

MRoush Over a year ago

Yeah, this was the issue. I think the other issue is also a print error. Thanks!

Nikos C. · Accepted Answer · 2013-08-02 19:40:22Z

1

This line:

std::cout << &myFSM.getTopNode();

probably prints the address of a temporary object, not the actual object in the vector. This will be true if you're not returning by reference but rather by value.

So it's not weird if the temporary happens to be created at the same location each time, since after the temporary dies, its location in memory is free to be used again later.

In order to get the actual object rather than a copy of it, getTopNode() needs to do:

Node& FSM::getTopNode()
{
    if (nodeList.empty()) {
        // Up to you how to handle this error.
    }
    return nodeList.back();
}

Of course, if your current getTopNode() implementation actually already returns a pointer:

Node* FSM::getTopNode()

then your problem is that you're printing out the address of the pointer rather than the pointer itself. In that case you should print with:

std::cout << myFSM.getTopNode();

edited Aug 2, 2013 at 19:40

answered Aug 2, 2013 at 19:09

Nikos C.

52.2k10 gold badges77 silver badges104 bronze badges

3 Comments

Mark Ransom Over a year ago

True, but it doesn't explain this assertion from the original question: "if I change anything in one of the Nodes stored by the FSM class, it changes it for all of them."

n. m. could be an AI Over a year ago

It doesn't. You cannot take an address of a temporary.

Nikos C. Over a year ago

@n.m. You can do that with some compilers.

John_West · Accepted Answer · 2013-08-02 20:00:34Z

Nothing happens similar to yours.

#include <iostream>
#include <vector>

class Node{
    private:
        std::vector<int> connects;
        int action;

    public:
        Node(int num){
            for(int i = 0; i < num; i++){
                connects.push_back(i);
            }

        }
    std::vector<int> getConn()
    {
        return connects;
    }
};

class FSM{
    private:
        std::vector<Node> nodeList;
    public:
        FSM(){}
    void addNode(int size){
        Node l(size);
        std::cout<<"temp_address "<<&l<<"\n";   
        nodeList.push_back(l);//use of default copy constructor
    }
    void printList(){
        std::vector<int> p;
        for (int i=0; i<nodeList.size(); i++)
        {
            std::cout<<"Node_arr_num "<<i<<" mem_address "<<&nodeList[i]<<"\nConnections:";
            p=nodeList[i].getConn();
            for (int j=0; j<p.size(); j++)
                std::cout<<" "<<p[j];   
            std::cout<<"\n";
        }
    }
};

int main()
{
FSM f;
f.addNode(5);
f.addNode(10);
f.addNode(3);
f.printList();
return 0;
}

Result:

temp_address 0xbfea7660
temp_address 0xbfea7660
temp_address 0xbfea7660
Node_arr_num 0 mem_address 0x8dab098
Connections: 0 1 2 3 4
Node_arr_num 1 mem_address 0x8dab0a8
Connections: 0 1 2 3 4 5 6 7 8 9
Node_arr_num 2 mem_address 0x8dab0b8
Connections: 0 1 2

Be careful with adding nodes later, when your app will grow. Temporary l object (ore your Node(obs)) must be copied with explicit copy constructor of class Node if Node will be more complex (contains fields with dynamic allocated memory).

Collectives™ on Stack Overflow

Class creating multiple objects of another class at the same memory location (C++)

3 Answers 3

3 Comments

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related