0

Suppose I have a vector of data generated from some API. It's for plotting only.

vector<double> LineData = generateData();

I also have a class that do the plotting and manage the data

class LineChart
{
public:
    LineChart(vector* data):m_data{data}{}
    vector<double> *m_data;
    void plot();
};

LineChart lc {&LineData}

Since the vector is only for plotting, I want the class to take the full ownership. So I want to change the raw pointer to a unique_ptr.

Normally, a unique_ptr is created from new or make_unique,instead of an existing object. I could make a copy of the vector, then move it to the class.

unique_ptr<vector> data = make_unique<vector>(LineData)

But there is unnecessary overhead to copy the vector, since I have the data already. Also, the original vector becomes duplicated on the stack and does nothing.

So in situation like this, are there some efficient ways to pass the ownership of existing object to the class? What's the best practice?

qiu
  • 13
  • 4
  • 3
    move `m_data` to private and it will be managed by that class – BЈовић Jul 12 '23 at 15:02
  • 3
    You may not need a pointer here. C++ allows things to be moved. – tadman Jul 12 '23 at 15:02
  • `std::make_unique>(std::move(LineData))`? – Jarod42 Jul 12 '23 at 15:09
  • whats the actual problem you are trying to solve with the (smart) pointer? `vector LineData` has automatic storage duration, ie nothing to be managed manually, you can copy/move it to a `vector` (no pointer) member, still nothing to be mangaged by a smart pointer. – 463035818_is_not_an_ai Jul 12 '23 at 15:14
  • 2
    `LineChart(std::vector data): m_data{std::move(data)} {}` followed by `LineChart lc {std::move(LineData)};` and you're done. – Evg Jul 12 '23 at 15:15
  • 1
    I would use `LineChart(std::vector&& data) : m_data(data){}` And then let the client code call the constructor like this `LineChart{std::move(LineData)};` This should avoid the extra copy of the constructor above. – Pepijn Kramer Jul 12 '23 at 15:19
  • 2
    This is a very dangerous problem in C++, it's called "Pointless Use Of Pointers". It does nothing but create trouble, and bugs. Nothing in the shown code requires the use of pointers, and can be done better, and safely, using proper move semantics. Correctly implemented move semantics carry no more "overhead" than merely swapping a few pointers around, but the C++ library will do it for you, correctly and safely. Just say "no" to Pointless Use Of Pointers! – Sam Varshavchik Jul 12 '23 at 15:21
  • @PepijnKramer Note that with `&&` you still need `std::move` in `m_data(data)`. – Evg Jul 12 '23 at 15:21
  • @EvgsupportsModeratorStrike Yup forgot that. – Pepijn Kramer Jul 12 '23 at 16:52

1 Answers1

8

There is no reason to use any kind of pointer here, just move the data into place. No copying of the vector happens.

class LineChart
{
public:
    LineChart(vector data):m_data{std::move(data)}{}
    vector<double> m_data;
    void plot();
};

LineChart lc {std::move(LineData)};

You could also initialise directly without using the LineData variable

LineChart lc {generateData()};

Here std::move is not necessary because the return value of the generateData function is a temporary and will be moved automatically.

john
  • 85,011
  • 4
  • 57
  • 81