Search code examples
c++oopvectorcopy-constructordefault-copy-constructor

Copying objects with all elements in C++! (Constructors and Assignment, best practice?)


I rummaged through SO and learned a lot regarding default constructors, copy constructors, objects assignment, smart pointers, shallow/deep copy and their relationships with dynamic memory allocation (e.g. This, This, That and ...). However, I'm still fuzzy on drawing a conclusion on what the best practice is to handle copying objects elements such as vectors (or list).

I learnt STL vector, in particular, handles this by its default copy constructor and best practice in this case is to not manage resources yourself. But it seems I'm understanding something wrong.

My effort before asking: I was also able to resolve this with passing the objects by reference but I ended of having too many deference operators (i.e. **).

What's the best practice here for simple small objects such as the one in the following code? Elements in vectors are not being copied properly. (I'd not be surprised if I'm doing extremely simple mistake here. Also, not using raw/shared/smart pointers is preferred if possible).

#include <iostream>
#include <vector>
using namespace std;

class A{
    public:
    int id;
    A(int id_):id(id_){}
    vector<A> childs;
};

int main()
{
    A a0(0), a1(1);

    a0.childs={a1}; //node0.childs.push_back(node1);
    a1.childs={a0}; //node1.childs.push_back(node0);

    cout << a0.childs.size() << endl; // 1
    cout << a1.childs.size() << endl; // 1
    cout << a0.childs[0].childs.size() << endl; // expecting 1 but 0
    //Probably since they're not pointing to the same address of memory
    //I was hoping vector handle this by itself (was told best practice in this case is to not manage resources yourself)

    return 0;
}

Solution

  • I think I understand what you are trying to achieve but if the objective is learning, then I strongly recommend you understand why, what you are expecting to happen, isn't happening. Before you move on to finding a "workaround" to achieve what you are trying to achieve.

    To better understand, it might help to write simplified code that demonstrates the same behavior. What you have written is more-or-less equivalent to:

    struct A {
        int childCount = 0;
    };
    
    int main() {
        A a1;
        std::vector<A> vecA{a1};
        a1.childCount = 1;
        std::cout << vecA[0].childCount<< "\n"; // What do you expect here?
    }
    

    which is equivalent to:

    A a1;
    A copyOfA1 = a1;
    a1.childCount= 1;
    std::cout << copyOfA1.childCount << "\n"; // What do you expect here?
    

    which is equivalent to:

    int a1 = 0;
    int copyOfA1 = a1;
    a1 = 1;
    std::cout << copyOfA1 << "\n";  // What about here?
    

    a0 holds a separate copy of a1 not a reference to a1 so if you make a change to the original a1, the copy of a1 held in a0 does not change.

    EDIT: As for how to achieve what you want to achieve. I'm assuming that A should not own its children. You want it to contain non-owning references to the As that are held elsewhere. Unfortunately, a std::vector cannot hold a C++ reference. The std::vector could hold a raw pointer but you specifically asked to not use raw pointers.

    An alternative, is a std::reference_wrapper<A> which is something that behaves a bit like a C++ reference but is assignable so that it can be used in a std::vector. You could hide the std::reference_wrapper by providing a member function to get access to a child by index:

    #include <iostream>
    #include <vector>
    #include <functional>
    
    struct A {
        int id;
        A(int id_):id(id_){}
        std::vector<std::reference_wrapper<A>> childs;
        A& at(size_t index) { return childs[index]; }
    };
    
    int main()
    {
        A a0(0), a1(1);
    
        a0.childs={a1};
        a1.childs={a0};
    
        std::cout << a0.childs.size() << "\n";
        std::cout << a1.childs.size() << "\n";
        std::cout << a0.at(0).childs.size() << "\n";
    }
    

    Live demo.

    But to be clear, std::reference_wrapper is basically just a wrapper around a raw pointer, it is up to you to ensure that object it is pointing to is still alive.

    Edit2: As requested, here is a version that uses a vector of raw pointers:

    #include <iostream>
    #include <vector>
    
    struct A {
        int id;
        A(int id_):id(id_){}
        std::vector<A*> childs;
        A& at(size_t index) { return *childs[index]; }
    };
    
    int main()
    {
        A a0(0), a1(1);
    
        a0.childs={&a1};
        a1.childs={&a0};
    
        std::cout << a0.childs.size() << "\n";
        std::cout << a1.childs.size() << "\n";
        std::cout << a0.at(0).childs.size() << "\n";
    }
    

    Live demo.

    Note that you have to take the address of a1 using & when initializing the vector.