1

I have a class graph which has a member:

std::vector<std::unique_ptr<layer>> _layers;

I understand that the nature of std::unique_ptr is to be non-copyable. My graph::graph(const graph & rhs); copy constructor produces a compilation error, because my ordinary implementation violates the nature of std::unique_ptr.

graph::graph(const graph & rhs)
: _layers(rhs._layers) {}

If I do a std::move then I will be violating the constness of param rhs, which is not what I want. I know I can make deep copies (e.g., create new unique pointers for each object stored in rhs._layers) but is there some elegant way of doing this, other than iterating each item in rhs._layers and allocating a new unique pointer?

gcc/g++ is c++11 not c++14

My current solution is deep copying the objects and allocating new pointers:

graph::graph(const graph & rhs)
{
   for (const auto & ptr : rhs._layers)
       _layers.emplace_back(std::unique_ptr<layer>(new layer(*ptr)));
}
Ælex
  • 14,432
  • 20
  • 88
  • 129
  • This had already been discussed here: http://stackoverflow.com/questions/16030081/copy-constructor-for-a-class-with-unique-ptr – Bobul Mentol Jan 11 '16 at 16:43
  • Uneducated guess: Would this be a good use case for `weak_ptr`s having a view to `shared_ptr`s? – Katana314 Jan 11 '16 at 16:43
  • @Katana314 I don't know TBH, but those pointers are not to be seen or accessed outside the class. I don't mind copying objects (hence the deep copy). What would the advantages of `weak_ptr` be in this scenario? – Ælex Jan 11 '16 at 16:46
  • @Alex Unlike a shared_ptr, that `_layers` variable would still be the owning reference to the actual `layer`s. If it's destroyed, the objects are destroyed. The disadvantage is, each access of `layer` from a `weak_ptr` would need to be inside a conditional statement to confirm the layer has not yet been destroyed. Also, there is no more compiler guarantee that the previously-unique pointer is not getting copied into another shared_ptr (so it would be up to you to safeguard it). – Katana314 Jan 11 '16 at 16:50
  • @Katana314 that actually makes sense, I don't know how I would implement it though. Care to provide a small example? – Ælex Jan 11 '16 at 16:53
  • 1
    If you don't want move semantics, perhaps unique_ptr is not the best choice. – n. m. could be an AI Jan 11 '16 at 17:00

3 Answers3

7

The most elegant solution would use the STL:

graph::graph(const graph & rhs)
{
    _layers.reserve(rhs._layers.size());
    std::transform(
        std::begin(rhs._layers),
        std::end(rhs._layers),
        std::back_inserter(_layers),
        [](const std::unique_ptr<layer>& uptr) {
            return std::unique_ptr<layer>{uptr ? new layer{*uptr} : nullptr};
        });
}

std::transform, std::back_inserter

Brian Rodriguez
  • 4,250
  • 1
  • 16
  • 37
  • Don't have c++14, and this is identical to what I'm already doing. I guess I need template specialization for my type. – Ælex Jan 11 '16 at 16:53
  • @Alex STL should be preferred to raw loops, but yes this is identical. Updated with a c++11 friendly version. – Brian Rodriguez Jan 11 '16 at 16:57
  • @BrianRodriguez If I have other members of class, I should copy this all in this copy-constructor. Right? – Max Aug 31 '18 at 07:09
  • 1
    @Max yep, each `std::unique_ptr` collection in your class should have these same 2 function calls made on them. – Brian Rodriguez Aug 31 '18 at 13:41
1

You can also review the possibility to use the vector of std::shared_ptr instead of std::unique, in this case you can avoid real copying

M. Bogdanov
  • 86
  • 2
0

You can't make a copy but you can make a move constructor:

graph::graph(graph && rhs)

You should be able to move the _layers vector in there and then you should be able to move graph instances.

If this is not what you want then you will have to abandon the use of unique_ptr or add a layer of indirection to the _layers vector if that can be shared among graph instances.

Edit: An alternative solution would be to not use pointers in the vector at all but emplace the instances into the vector directly. This way the vector would be copied with all the contents naturally.

Fozi
  • 4,973
  • 1
  • 32
  • 56
  • this violates the whole point of having a const object parameter. Besides I am not interested in *moving* but *copying*. – Ælex Jan 11 '16 at 16:50
  • @Alex then you should not be using `std::unique_ptr` in the first place. Is there a reason you are using a pointer here at all? Could you just emplace the instances in the vector? – Fozi Jan 11 '16 at 17:13