When to Sink a unique_ptr

Question

I'm relatively new to C++ and want to adopt modern practice. I've been trying to understand when it's best to sink a unique_ptr, here is some code I have:

class SomeClass
{
    ...

private:
    unique_ptr<QStaticText> _text;
}


{
    ...

    void SomeClass::setText(unique_ptr<QStaticText> newText)
    {
        _text = move(newText);
    }

    void SomeClass::setText(const QStaticText& newText)
    {
        _text = make_unique<QStaticText>(newText);
    }

    ...
}

Should I prefer one over the other, either or another?

The second one should be preferred. But it's a bit unclear why you can't have a `QStaticText` member directly. — πάντα ῥεῖ, Jan 23 '16 at 11:27
Your example sucks, but the general idea is that you accept or return a `unique_ptr` when you want to transfer ownership. This is especially important when the resource being transferred can't be copied (uncopyable, expensive, full type unknown), which is also what makes your example so bad. If you can copy and assign a `QStaticText`, don't use pointers. In general, avoid using `new`, although it might be slightly different with Qt. — Ulrich Eckhardt, Jan 23 '16 at 11:50
Have a read of Herb Sutter: Smart Pointer Parameters: http://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/ — Richard Critten, Jan 23 '16 at 12:42

score 1 · Answer 1 · edited May 23 '17 at 11:59

1

Referring to the interface of QStaticText, it's unclear why you're using pointers (smart or otherwise) at all. (I believe panta rei noted this in a comment too.) Have a look at Why Should I Use a Pointer Rather Than the Object Itself.

In this case, it might be better to have something like this:

class SomeClass
{
    ...

private:
    QStaticText _text;
};


template<class Text>
void SomeClass::setText(const Text &newText)
{
    _text = QStaticText(newText);
}

Note the following:

QStaticText is just optimized for infrequent changes.
It is constructible by at least two different types.

It's hard to see what you gain by your present scheme. For each update, you're creating multiple objects anyway, and you can't reuse them (you're moving the content out of them).

edited May 23 '17 at 11:59

Community

1
1

answered Jan 23 '16 at 11:46

Ami Tavory

74,578
11
141
185

Great, so a different approach would be: class SomeClass { ... private: QString _text; } { ... void SomeClass::setText(const QString& newText) { _text = QStaticText(newText); } ... } – Peter Spencer Jan 23 '16 at 14:53
@PeterSpencer Yes; what I wrote is a slight generalization of that, as it can take either a `QString` or a `QStaticText`. As far as your original q. goes, though - I think your current suggestion is better than the original one. – Ami Tavory Jan 23 '16 at 15:02

When to Sink a unique_ptr

1 Answers1