Add an item in a container of smart pointers
c++, containers, smart-pointers, unique-ptr
Solution
I vote for:
bool Add(std::unique_ptr<Item> item);
Reasons:
It's clear from the function signature that the client needs to pass ownership of the object to the `MyContainer`. If you choose option 1 instead, it's still not clear if the client should `delete` the object themselves or not, or even if they should be passing a dynamically allocated object.
Client is forced to explicitly transfer ownership with `std::move` if they already have the object managed by a named `std::unique_ptr`. They won't accidentally lose ownership. Option 3 does not clearly express that it's going to take ownership.
When we have `std::make_unique` (N3588), the method for adding an element will be:
container.Add(std::make_unique<Item>());
This avoids using `new` and improves exception safety in some situations.
The issue you've given for derived objects is not really a problem. You'll get a compile-time error if you do it incorrectly.
If the interface changes to use a different type of smart pointer, the client will want to know. They don't want to continue passing objects thinking that they're passing ownership if in reality they're sharing it. They'll especially want to know if the opposite happens.
Problem
Several ways to add an item in a container of smart pointers. I am wondering which way you will go for. ``` class MyContainer { private: std::vector<std::unique_ptr<Item>> mItems; public: bool Add(Item* item); // This is Way 1 // // Advantages: // - Easy to add derived items, such as Add(new DerivedItem); // - No interface change if smart pointer type changes to such as shared_ptr; // // Disadvantages: // - Don't explicitly show the item to add must be allocated on heap; // - If failed to add, user has to delete the item. bool Add(std::unique_ptr<Item> item); // This is Way 2 // Disadvantages and advantages are reversed from Way 1. // Such as to add derived item, Add(std::unique_ptr<Item>(new DerivedItem)); // | // easy to write DerivedItem here for an error bool Add(std::unique_ptr<Item>& item); // This is Way 3 // Similar to Way 2, but when failed to add, item still exist if it is a // reference of outer unique_ptr<Item> }; ``` I personally go for Way 1. Any more advantages for Way 2 and 3 or disadvantages of Way 1 that I should go for 2 or 3? sftrabbit gives many good points. In the following common case. How to use Way 2 or 3 to do it easily? User uses a dialog to generate a new derived item. It is put on `std::unique_ptr<DerivedItem> item`. When click 'OK' button, it is added to the container. If failed to add, go back to the dialog for an edit.