use iterator ptr to update vector of unique_ptr

Question

Code as follows:

#include "MyObject.h"
#include <vector>
#include <memory>

class MyCollection {
private:
    std::vector<std::unique_ptr<MyObject*>> collection;
public:
    MyCollection();
    virtual ~MyCollection();

    int insert(MyObject* newValue);

};

int MyCollection::insert(MyObject* newValue) {
    if (collection.empty()) {
        collection.push_back(move(make_unique<MyObject*>(newValue)));
        return 0;
    }
    int index = collection.size()-1;

    collection.resize(collection.size()+1);
    vector<unique_ptr<MyObject*>>::reverse_iterator pos = collection.rbegin();

    for ( ; (index >= 0) && (pos+1) != collection.rend() && stringToUpper((*(pos+1)->get())->getObjectName()) > stringToUpper(newValue->getObjectName()); ++pos) {
        pos = (pos+1);
        index--;
    }

    pos = ?newValue; // How do I do this?
        //pos->reset(move(make_unique<MyObject*>(newValue)));

    return index+1;
}

make_unique() implementation taken from http://scrupulousabstractions.tumblr.com/post/37576903218/cpp11style-no-new-delete

My question is there a way to do what I'm attempting with the assignment to the reverse_iterator (pos = newValue)? One of my pitiful attempts is shown in the commented code.

Thanks!

Answer 1

Firstly, as others have pointed out, you want a vector<unique_ptr<MyObject>> not vector<unique_ptr<MyObject*>>. It is fine to have a unique_ptr containing an abstract class (make sure the base class has a virtual destructor). You can implicitly cast from a unique_ptr containing a derived class.

Ideally I think MyCollection::insert should take a unique_ptr not a raw pointer so that the calling code creates objects using make_unique in the first place but let's leave it like it is for now.

I think you have a bit of confusion with make_unique. make_unique is designed to create an object and wrap it in a unique_ptr in one go, safely. Your MyCollection::insert doesn't really need to use make_unique because the object is already created. unique_ptr has a constructor that takes a raw pointer so you can create one directly from the raw pointer.

You can then push the unique_ptr onto the collection or replace unique_ptrs in the collection with new unique_ptrs fine:

class MyObject {
  public:
    virtual ~MyObject() = 0
};
MyObject::~MyObject() {}

class SimSolverObject : public MyObject {
};

class MyCollection {
  private:
    std::vector<std::unique_ptr<MyObject>> collection;
  public:
    void insert(MyObject* newValue);
};

void MyCollection::insert(MyObject* newValue) {
  //...

  // if we want to add to the collection
    collection.push_back(std::unique_ptr<MyObject>(newValue));

  // if we want to replace at iterator pos in collection
    *pos = std::unique_ptr<MyObject>(newValue);
}

// calling code
MyCollection mc;
MyObject* newValue = new SimSolverObject();
mc.insert(newValue)

If you do decide to change MyCollection::insert to take a unique_ptr it would look something like this:

void MyCollection::insert(std::unique_ptr<MyObject> newValue) {
  //... 

  // if we want to add to the collection
  collection.push_back(std::move(newValue));

  // if we want to replace at pos 
  *pos = std::move(newValue);
}

Edit: Your for loop looks a bit suspicious. I am not quite sure what you are trying to do but are you sure you want to increment the iterator twice? Once in the body of the for and once in the loop expression? I suspect the iterator is skipping over your condition and going out of bounds of the vector. When it hits the index condition you may be left with an invalid iterator.