1

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;
}
Damore Su
  • 13
  • 3
  • 2
    A good, recent C++ book that covers this subject matter will have a far more complete explanation, and provide detailed information on the new features of the current C++ standard, such as move semantics, far more thoroughly than a short, terse answer on stackoverflow.com. Stackoverflow.com is not a substitute for a good C++ book. If you'd like to learn C++ [the only way to do so is with a good book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). There is no Magic Button Of Knowledge here, and no instant gratification. Learning this takes time. – Sam Varshavchik Oct 22 '18 at 04:56
  • @SamVarshavchik So true. I tried to narrow my question down to STL containers, hoping to get an advice. – Damore Su Oct 22 '18 at 05:00
  • 2
    It's not entirely clear what the "this" is that you're trying to resolve. Everything is being copied properly – I suspect that you're expecting some kind of reference semantics. Get a good introductory book and try not to learn everything at once. – molbdnilo Oct 22 '18 at 05:26
  • @molbdnilo I want to copy STL containers content, as an element of objects, when I copy them. Vector, in particular. – Damore Su Oct 22 '18 at 05:38
  • Why were you expecting `a0.childs[0].childs` to have a size of 1? It might be good to draw on what you have learned about copy constructors and identify each line where an `A` is copied. So then, how many different `A` objects do you have? – Chris Drew Oct 22 '18 at 05:38
  • @ChrisDrew Because it's initially zero but when I added `a0`, as a child, to `a1`. I was expecting to see this effect through `a0.childs[0].childs.size()` after doing this: `a1.childs={a0}`. – Damore Su Oct 22 '18 at 05:42
  • 1
    @DamoreSu but you are not really "adding `a1`, as a child, to `a0`" are you. You are adding a *copy* of `a1`, as a child, to `a0`. It is best not to think of it as `a1` anymore. It is a completely separate, unnamed `A` object that is owned by `a0`. – Chris Drew Oct 22 '18 at 05:49
  • @DamoreSu You *are* copying the contents. If you didn't copy, you would see the behaviour you're expecting. (If you're coming from some other language - Java/C#/JavaScript/Python/... - there are many things you need to unlearn.) – molbdnilo Oct 22 '18 at 08:58

2 Answers2

0
a0.childs={a1}; //a1.childs is empty here, so a0.childs will be empty too
a1.childs={a0}; //a0.childs contains one element here and so will a1.childs

So

 cout << a0.childs[0].childs.size() << endl; // This size will be 0

But

cout << a1.childs[0].childs.size() << endl; // This will contain one element and the output shows the same.

Output:

a.exe
a0.childs.size():1
a1.childs.size():1
a0.childs[0].childs.size():0
a1.childs[0].childs.size():1
P.W
  • 26,289
  • 6
  • 39
  • 76
  • Yes. I conveyed myself wrong. What is the right solid way to handle it? (liek best practice). Can you add your code please (not the workaround tho) – Damore Su Oct 22 '18 at 05:27
  • Not sure what exactly you are trying to do here. You can fill in the elements of a0 and a1 and use std::swap to exchange the contents. – P.W Oct 22 '18 at 05:41
  • I was expecting to see the effect of `a1.childs={a0}` through `a0.childs[0].childs.size()`. – Damore Su Oct 22 '18 at 05:46
  • 1
    This will not happen because the assignments involve copies of the objects and not the objects themselves. Changing the object AFTER the assignment will not affect what it has been assigned to. – P.W Oct 22 '18 at 05:57
0

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.

Chris Drew
  • 14,926
  • 3
  • 34
  • 54
  • This level of understanding is amazing. You earned my respect, sir. I loved the surgery you did on my poorly explained question. I understood your workaround, especially now I know where the problem is coming from with your simple yet brilliant example. So, if not wrong, can I now conclude that the correct way to handle such cases is to use `pointers` or `references`? (perhaps using `vector* childs` or `vector childs`). Can you please shed more lights by adding code using raw pointers too? (if it's the right way to go) – Damore Su Oct 22 '18 at 10:43
  • My confusion originated from [this post](https://stackoverflow.com/questions/31686973/c-copy-constructor-vs-overloaded-assignment-vs-constructor-best-practices) where it says "best practice in copying objects with vectors is to not manage resources yourself." – Damore Su Oct 22 '18 at 10:45
  • I just came across [this](https://stackoverflow.com/a/27463966/10538564)... seems very relevant. Actually I'm trying to build almost the same kind of reusable graph-theory code for my own practice purposes. – Damore Su Oct 22 '18 at 11:01
  • @DamoreSu I've provided a version that uses `vector`. – Chris Drew Oct 22 '18 at 11:08
  • I see that. Thank you sir. But is this the best way to go? Please go ahead and update your code without any restriction if there is a better way to go (i.e. any types of pointers if needed, e.g. `unique_ptr` or `shared_ptr`). I greatly appreciate your contribution and will continue educate myself. – Damore Su Oct 22 '18 at 11:09
  • [This](https://stackoverflow.com/questions/27460377/why-is-using-vector-of-pointers-considered-bad/27463966#27463966) has made me scratch my head... :/ – Damore Su Oct 22 '18 at 11:15
  • When I revisited your answer, I realized that this is what I did initially by passing object by references (and dereferencing them in your `at()`). Sorry for too many questions, I just fully digested your answer. So, reference is the way to go? or there is a better way....? Thanks a ton. – Damore Su Oct 22 '18 at 11:24
  • @DamoreSu I quite like using `std::reference_wrapper` but its use is not that common and the difference between that and a raw pointer is probably just a matter of style. – Chris Drew Oct 22 '18 at 11:40