Passing a vector of unique_ptr to an object. the vector becomes a member variable. correct approach?

Question

One object I will call "Owner", has clear ownership of a vector of data objects during its lifetime.

These are stored as a vector of unique_ptr.

One object/class, called "Output", needs to look at these data objects in many different methods, and therefore some kind of reference / pointer / variable is a member variable of "Output".

Output receives the vector of data objects in its constructor.

I have thought of three ways to achieve this. What would be considered the best way?

Option 1 - "output" object stores data vec as const reference:

class Output {
    // output wants the data:
 public:
    Output(std::vector<std::unique_ptr<Data>> const & in)
        : my_lot_of_data(in) {
    };
    std::vector<std::unique_ptr<Data>> const & my_lot_of_data;
}

which is instantiated by the "Owner" with:

 data_vec_.push_back(std::unique_ptr<Data>(new Data));
 /* stuff happens to data */
 Output output(data_vec_);

Option 2 - "output" object stores data vec as const pointer:

class Output {
    // output wants the data:
 public:
    Output(std::vector<std::unique_ptr<Data>> const * in)
        : my_lot_of_data(in) {
    };
    std::vector<std::unique_ptr<Data>> const * my_lot_of_data;
}

which is instantiated by the "Owner" with:

 data_vec_.push_back(std::unique_ptr<Data>(new Data));
 /* stuff happens to data */
 Output output(&data_vec_);

Option 3 - "output" object receives raw pointers:

class Output {
    // output wants the data:
 public:
    Output(std::vector<Data*> in)
        : my_lot_of_data(in) {
    };
    std::vector<Data*> const my_lot_of_data;
};

which is instantiated by the "Owner" with:

 data_vec_.push_back(std::unique_ptr<Data>(new Data));
 /* stuff happens to data */
 std::vector<Data*> data_as_raw;
 data_as_raw.resize(data_vec_.size());
 std::transform(data_vec_.begin(), data_vec_.end(), data_as_raw.begin(), [](std::unique_ptr<Data> const & x) {return x.get();});
 Output output(data_as_raw);

Additional queries: In option 1 and 2 is it clear that Output doesnt have ownership of the data, even though it is stored as unique_ptrs? Is option 3 to messy at the call site? It takes 3 more lines to achieve the same result.

What is best practice here?

Answer 1

If all you need is to "read some of the values, and then [create] various output files" I would just make it a function that takes by const-ref:

void output(std::vector<std::unique_ptr<Data>> const& data) {
    // stuff
}

I prefer const& to const* for usage semantics (data[0] vs (*data)[0]), and definitely prefer both to passing in the raw data - don't give up your explicit ownership for nothing (and in this case it's not even for convenience given that it's pretty annoying to construct a vector<Data*> anyway)

Answer 2

Since you are using unique_ptr, you don't intend to share this data with anything that could persist longer than Owner, so a simple const reference should be good. I would recommend a nice typedef:

typedef std::vector<std::unique_ptr<Data>> OwnerDataSet;
Output(const OwnerDataSet &in)

The advantage of approach 1 is that it is simple and clear. The others just complicate it for no apparent reason.

The function of unique_ptr is to delete the new Data when the std::vector is destructed. The alternative here would be to copy an instance of Data instead of calling new. If you don't need to work with pointers, then you don't need special handling like unique_ptr to keep them safe.

typedef std::vector<Data> OwnerDataSet;
OwnerDataSet results;
Data match = findoneresult();
results.push_back(match); // copy something from Owner
Output (results);

And to go a step further, it is not clear from your examples why you are maintaining the std::vector outside of the Output class. Since you are calling std::unique_ptr(new T) on everything being passed, I suspect you only use it with Output, so you could do this:

class Output : public std::vector<Data> {
    void PrintToScreen();
    void WriteToDatabase();
    void OrWhatever();
};

Output x;
Data match = findoneresult();
x.push_back(findoneresult());
x.PrintToScreen();