I have a class "Token" and a class "ASTNode". ASTNode is a tree representation of a list of "Token"'s.
#define INTEGER "INTEGER"
#define ARRAY "ARRAY"
class Token {
private:
std::string type_t;
std::string value_t;
public:
Token() {
type_t = "";
value_t = "";
}
Token(std::string type, std::string value) {
type_t = type;
value_t = value;
}
~Token(){};
std::string value() {
return value_t;
}
std::string type() {
return type_t;
}
std::string str() {
return ("Token(" type_t ", " value_t ")");
}
};
Class ASTNode
class ASTNode {
public:
std::vector<ASTNode*> child;
Token _token;
ASTNode() {};
ASTNode(Token token) {
_token = token;
}
~ASTNode() {};
void make_child(ASTNode _node) {
ASTNode *temp = new ASTNode(_node.token());
temp->child = _node.child;
child.push_back(temp);
}
Token token() {
return _token;
}
};
Now i have a std::map<std::string, ASTNode>
where i want to store these "Array" nodes
std::map<std::string, ASTNode> ARRAYS;
// create a node
ASTNode arr1(Token(ARRAY, "a"));
arr1.make_child(Token(INTEGER, "1"));
arr1.make_child(Token(INTEGER, "2"));
arr1.make_child(Token(INTEGER, "3"));
// store as ARRAYS["a"]
ARRAYS.insert(std::pair<std::string, ASTNode>("a", arr1));
// create another node
ASTNode arr2(Token(ARRAY, "b"));
arr2.make_child(Token(INTEGER, "4"));
arr2.make_child(Token(INTEGER, "5"));
arr2.make_child(Token(INTEGER, "6"));
// store as ARRAYS["b"]
ARRAYS.insert(std::pair<std::string, ASTNode>("b", arr2));
My problem is when i want to create a new array to be a copy of an existing array. Lets say i want to create array c
to be the same as array a
ARRAYS.insert( std::pair<std::string, ASTNode>("c", arr1) );
Now i want to change the value of the 1st "array element" of the copy stored as c
.
ASTNode tmp = ARRAYS["c"];
tmp.child[0]->_token = Token(INTEGER, "0");
//ARRAYS["c"] = tmp;
ARRAYS.insert(std::pair<std::string, ASTNode>("c", tmp));
but instead of only array c
has been modified, array a
also has been. How to change this so i can change only the copy of it?
CodePudding user response:
Just create a deep copy constructor of ASTNode and everything works well:
ASTNode(ASTNode const & other) {
_token = other._token;
for (auto ochild: other.child)
child.push_back(new ASTNode(*ochild));
}
You may also wish to add similar assignment operator =
method implementation (but it is not needed to fix your current error, only above copy constructor is needed), assignment needs Clear()
method for cleanup, which can be used in destructor also for freeing memory:
ASTNode & operator = (ASTNode const & other) {
Clear();
_token = other._token;
for (auto ochild: other.child)
child.push_back(new ASTNode(*ochild));
return *this;
}
void Clear() {
for (auto c: child)
delete c;
child.clear();
_token = Token();
}
~ASTNode() { Clear(); }
Full working code:
#include <string>
#include <vector>
#include <map>
#include <iostream>
#define INTEGER "INTEGER"
#define ARRAY "ARRAY"
class Token {
private:
std::string type_t;
std::string value_t;
public:
Token() {
type_t = "";
value_t = "";
}
Token(std::string type, std::string value) {
type_t = type;
value_t = value;
}
~Token(){};
std::string value() {
return value_t;
}
std::string type() {
return type_t;
}
std::string str() {
return ("Token(" type_t ", " value_t ")");
}
};
class ASTNode {
public:
std::vector<ASTNode*> child;
Token _token;
ASTNode() {};
ASTNode(Token token) {
_token = token;
}
ASTNode(ASTNode const & other) {
_token = other._token;
for (auto ochild: other.child)
child.push_back(new ASTNode(*ochild));
}
ASTNode & operator = (ASTNode const & other) {
Clear();
_token = other._token;
for (auto ochild: other.child)
child.push_back(new ASTNode(*ochild));
return *this;
}
void Clear() {
for (auto c: child)
delete c;
child.clear();
_token = Token();
}
~ASTNode() { Clear(); }
void make_child(ASTNode _node) {
ASTNode *temp = new ASTNode(_node.token());
temp->child = _node.child;
child.push_back(temp);
}
Token token() {
return _token;
}
};
int main() {
std::map<std::string, ASTNode> ARRAYS;
// create a node
ASTNode arr1(Token(ARRAY, "a"));
arr1.make_child(Token(INTEGER, "1"));
arr1.make_child(Token(INTEGER, "2"));
arr1.make_child(Token(INTEGER, "3"));
// store as ARRAYS["a"]
ARRAYS.insert(std::pair<std::string, ASTNode>("a", arr1));
// create another node
ASTNode arr2(Token(ARRAY, "b"));
arr2.make_child(Token(INTEGER, "4"));
arr2.make_child(Token(INTEGER, "5"));
arr2.make_child(Token(INTEGER, "6"));
// store as ARRAYS["b"]
ARRAYS.insert(std::pair<std::string, ASTNode>("b", arr2));
std::cout << "a token " << arr1.child[0]->_token.value() << std::endl;
ARRAYS.insert( std::pair<std::string, ASTNode>("c", arr1) );
ASTNode tmp = ARRAYS["c"];
tmp.child[0]->_token = Token(INTEGER, "0");
//ARRAYS["c"] = tmp;
ARRAYS.insert(std::pair<std::string, ASTNode>("c", tmp));
std::cout << "a token " << arr1.child[0]->_token.value() << std::endl;
}
Output before change:
a token 1
a token 0
Output after change:
a token 1
a token 1
CodePudding user response:
Solution
class ASTNode {
public:
std::vector<ASTNode*> child;
Token _token;
ASTNode() {};
ASTNode(Token token) {
_token = token;
}
ASTNode(const &ASTNode other) {
_token = other._token;
for (ASTNode* node : other.child) { make_child(*node); }
}
~ASTNode() {
// YOU REALLY SHOULD FREE YOUR VECTOR HERE!
// for example:
for (ASTNode* node : child) { delete node; }
};
void make_child(ASTNode _node) {
ASTNode *temp = new ASTNode(_node.token());
temp->child = _node.child;
child.push_back(temp);
}
Token token() {
return _token;
}
};
Explanation
When you copy an ASTNode
(i.e. with ASTNode tmp = ARRAYS["c"];
) you are actually copying the memory hole by the elements of the struct.
Token
is simply a couple of strings, the std::string
copy constructor ensure you will have two different instances of the same string.
std::vector<ASTNode*>
is in the underlying an array, but the elements are copyied in the new memory when the copy constructor is called. When you copy the elements of this array, you are copying a number which stores the address of the memory of an object, but not the object itself. That address is called pointer.
We can explain this by assuming a real case: imagine there are three houses in Time Square, Manhattan
, one red at number 0, one green at 1 and one blue at 2. When you hold a vector of pointers you are storing just the addresses: [ "Time Square, Manhattan, 0", "Time Square, Manhattan, 1", "Time Square, Manhattan, 2" ]
. Copying this vector will copy just the address, but the houses does not change. tmp.child[0]
and arr1.child[0]
are both the address "Time Square, Manhattan, 0"
which refer to the same red house.
When you instead hold a vector of instances (std::vector<ASTNode>
) you are not interested in the address but in the shape, color and physical aspect of the house. So you will copy the aspect of the red house but it can be placed for example at "Cooper Square, Manhattan, 0"
. In the aspects are the same houses, but they are two different buildings in two different places of Manhattan.
So by doing tmp.child[0]->_token = Token(INTEGER, "0");
you are actually saying: "The token of the house at Time Square, Manhattan, 0
is now Token(INTEGER, "0")
". Every other vector that has saved that address will "see" the same changes, because the building is changed in someway.
Now you have two options:
- Use
std::vector<ASTNode>
so the default copy constructor will copy the aspect of the house, but keeping two different buildings. - Make sure to create a copy of the building in a different place (memory location) so you can modify the new building without touching the old one (see solution).
NOTE: Always ensure you destroy the buildings you do not need anymore! (see solution destructor)