1

I had two matching vectors of unique_ptr. I decided to unify them by making one vector of structs containing two unique_ptr (the struct will also contain other items, eventually, hence this refactoring).

What is the best approach for me to add new items in this vector?

My current code is 

std::vector<DestinationObjects> destinations;
for (unsigned short &id: ids) {
    DestinationObjects d;

    d.transmitter = unique_ptr<Transmitter> (new Transmitter(id));
    d.controller = unique_ptr<Controller> (new Controller(id));

    destinations.push_back(d);
}

Of course this copies d, causing issues with unique_ptr. What is the best approach to fixing this?

Options I can conceive of, some of which I am not sure will work:

  1. Just switch to shared_ptr. (A simple find-replace, but feels like a cop-out).
  2. Write a move constructor for the struct that moves the unique_ptr. (Another thing to maintain that could go wrong, as I expand the struct).
  3. Push back an empty struct instance with null pointers for the unique_ptr and edit these in place once in the vector. (Fiddly syntax. Also I'm not sure this would even work).

Any ideas what would be another approach? Or why I should prefer one of the ones I have listed?

rod
  • 3,403
  • 4
  • 19
  • 25
  • 1
    What issues with `unique_ptr` do you think your code causes? – Oswald Nov 01 '13 at 13:58
  • 4
    The implicit move constructor should be fine, unless you've killed it by writing your own copy constructor for some reason. The type has to be movable anyway, since `vector` needs to move objects when it reallocates. – Mike Seymour Nov 01 '13 at 13:58
  • 4
    Just... move it? Or better, get yourself a constructor like a boss and emplace_back. – R. Martinho Fernandes Nov 01 '13 at 13:58
  • @Oswald: It can't be copied, so `push_back(d)` will fail to compile. – Mike Seymour Nov 01 '13 at 13:59
  • @MikeSeymour Aha! I think the compiler I'm currently testing with is failing to write an implicit move constructor (VC11). See here: http://stackoverflow.com/questions/10201659/why-is-this-copy-constructor-called-rather-than-the-move-constructor – rod Nov 01 '13 at 14:04
  • If the lifetime of the objects *in* the struct is supposed to be the same lifetime *of* the struct - and your use of unique_ptr suggests this might be so - then why aren't they just on the stack??? – Grimm The Opiner Nov 01 '13 at 14:17
  • @user1158692 Mainly because these objects are really quite large and I'm creating a substantial number of them. But I'm willing to acknowledge I may be making a mistake by not putting them on the stack. – rod Nov 01 '13 at 14:25
  • If you are storing them in a vector, then you only have one "on the stack" at any given time (the rest are in the vector whose space is on the heap). And if you use emplace_back instead of push_back, you won't even have that. – Nevin Nov 01 '13 at 14:31
  • @user1158692: As a follow-up to your and Nevin's points, I also start several threads with pointers to these objects. If I were to store the objects directly in the vector, I imagine I'd have to restructure my threads to take a pointer to the vector + an index (rather than a pointer to the object), if they were to reliably have access to these objects. Unless there's another trick I'm not aware of? – rod Nov 01 '13 at 17:31
  • @rod does the `vector` change size after you pass them off to the threads and before the threads finish? If not, their address (in the `vector`) will stay stable. – Yakk - Adam Nevraumont Nov 01 '13 at 20:24

1 Answers1

1

Simpley do a vec.emplace_back( std::move(d) ).

If (as mentioned in your comment) your compiler does not implement implicit move construtors, write your own move constructor. My advice in the future is whenever you have a problem with any C++11 feature and are asking a question, mention that you are using this compiler, as there is a pretty good chance that it's "C++11 support" will be an important issue.

If your compiler does not support any move constructor at all, stop using unique_ptr -- they are rather useless without move constructors. ;)

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524