Copy assignment implementation of a class which has a member object of that class.(c++)

Question

I am a new computing science student and not very experienced in c++. I've been googling for hours, and did not come across a solution(to be honest, i couldn't even find a good statement to search). Basically what i'm trying to learn here is how to implement a copy operator for a case like:

#include <string>
#include <vector>
class foo
{
    foo* parentObject;
    std::vector<foo*> childObjects;
    char a;
    int *b;
public:
    foo(char a, int* b) : a(a), b(b) {};
    foo();
    foo& operator =(const foo& other)
    {
        a = other.a;
        b = new int(*other.b);
        //parentobject???
        //pretty sure i can figure childObjects if i can manageparentObject,
        //but just in case there are some special stuff wanted to include.
        return *this;
    }
    friend void getElements(std::string fpath);
};

To go deeper, i want to read objects of the same type that are in a tree-like hierarchy(sort of like XML) from a formatted text file and have only one instance of the object i read in the memory(lets assume thats what getElements does), and the rest are pointers to those objects(we can think of this as how shortcuts work). That is because, i thought that would be the most efficient method since there can be a lot of objects, please correct me if i'm wrong.

But i also want to keep track of the structure so i decided to go with this parent/child design(again could be a bad choice, feel free to enlighten me). Because of the reading method i used(from parent to children), even though i can find a way to initialize parentObject, i can't simply initialize childObjects. Therefore i can't use a 'new constructor()' assignment(or maybe i can but i just don't know that i can).

If the reason behind my need of (shallow)copy operator is unclear, since i didn't initialize parentObject or childObjects, i need to assign them later on while reading the text document(it really feels like i shouldn't need one but i think i do.).

My actual code is really hard to read; since this is a project just to get better at c++, i didn't give importance to coding style) and it is longer so i created a simpler version. I really appreciate any comment so feel free to write one.

Edit: I couldn't even copy paste properly(initialization of b). Edit: Clearence.

`b(b)` in your member initialization list had better flag a warning from your compiler; you're initializing `int*` with `int`. Beyond that, understanding the [Rule of Three](http://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)) better is probably warranted. — WhozCraig, Mar 20 '15 at 14:54
http://stackoverflow.com/questions/10677394/how-to-make-a-deep-copy-of-this-constructor — NathanOliver, Mar 20 '15 at 14:59
I think you still didn't state the problem clear. do you want a deep copy or just shallow copy? — Griffan, Mar 20 '15 at 15:06
Shallow copy. I want both new copy and old pointer to point at the same object/memory location. — user3402183, Mar 20 '15 at 16:21
@user3402183: I think what you actually want is to disallow copies, and to only allow moves. — Mooing Duck, Mar 20 '15 at 16:31

score 2 · Answer 1 · answered Mar 20 '15 at 14:57

2

When you have an object that has member variables to define a tree of the objects, copy constructor or copy assignment operator will not be sufficient to deal with all cases. You may want to:

Copy just the contents of a node in the tree (a, and b in your case).
Copy one layer of parents and children.
Copy N layers of parents and children.
Copy N1 layers of parents and and N2 layers of children.
Copy the entire tree.

(1) above can be handled by a copy constructor or a copy assignment operator. For the rest, it's better to have extern functions that traverse the tree and make a copy of the right portion of the tree.

answered Mar 20 '15 at 14:57

R Sahu

204,454
14
159
270

I see, but i still don't understand how i would assign a value to the pointer. Does that mean i don't need to overload 'operator=' to execute: *myobject.parentObject = otherObject; – user3402183 Mar 20 '15 at 16:33
Sorry i hit enter and it posted the comment, i didn't know comments did that. – user3402183 Mar 20 '15 at 16:36
@user3402183, that seems to me to be the best you can do. Just copy `a` and `*b`, and leave the rest alone. – R Sahu Mar 20 '15 at 16:38
Oh, i only tried to implement one (edit:) that copies those layers because i thought i had to... But i'm happy i did so i could learn these things. – user3402183 Mar 20 '15 at 16:49
@user3402183, that's the right thing to do. Using the compiler generated copy constructor and copy assignment operator will be a problem for a `struct` like yours. – R Sahu Mar 20 '15 at 16:51

Mooing Duck · Accepted Answer · 2015-03-20T16:47:15.280

You've run into the logical problems of having an XML node copy itself. There is really no clear and intuitive way to go about copying an XML node that's part of a tree. And a rule of thumb in C++ is: if an operator doesn't act intuitively, make it a named function instead. In your case, I would make a foo& assign_from_content(foo&) function that acts like a copy assignment, and a static foo construct_from_content(foo&) that acts like a copy constructor, that only copies the content.

Now what to do with the copy constructor and copy assignment operators? delete them. And use move construction and move assignment instead.

foo& operator =(const foo& other)=delete;
foo& operator =(foo&& other) noexcept=delete;
{
    disconnect_from_tree();

    a = other.a;
    delete b;
    b = other.b;
    other.b = null;

    take_place_in_tree(other);       
    return *this;
}

//These are untested, they probably don't compile and have logical errors
void disconnect_from_tree() noexcept {
    if (parentObject) {
        auto it = std::find(parentObject->childObjects.begin(), 
                            parentObject->childObjects.end(), 
                            this);
        if(it != parentObject->childObjects.end())
            parentObject->childObjects.erase(it);
    }
    for(foo* child : childObjects)
        child->parentObject = nullptr;
    childObjects.clear();
}

//These are untested, they probably don't compile and have logical errors
void take_place_in_tree(foo& other) noexcept {
    childObjects = std::move(other.childObjects);
    parentObject = other.parentObject;     
    other.disconnect_from_tree();
    for(foo* child : childObjects)
        child->parentObject = this;  
    if (parentObject) {
        auto it = std::find(parentObject->childObjects.begin(), 
                            parentObject->childObjects.end(), 
                            this);
        if(it != parentObject->childObjects.end())
            parentObject->childObjects.erase(it);
        parentObject->childObjects.push_back(this);
    }
}

I actually thought about this but it was rather unclear to me if or how i could implement it. Thank you for your efforts. — user3402183, Mar 20 '15 at 16:58

Copy assignment implementation of a class which has a member object of that class.(c++)

2 Answers2