Unique_ptr, it's pants, i don't like it!

There. I’ve said it.

I’ve got a whole new category of errors where i’ve std::moved something into something else and then try and use the old, now null, pointer.

There’s too much typing.

Error messages are shite!

I miss ScopedPointer.

:frowning:

Going to stick another pin into my C++ standards committee voodoo doll now.

Have a good weekend!

3 Likes

Let’s start a support group @jimc :rofl:

1 Like

I think there’s you’re problem right there :wink:

2 Likes

Sounds like that instead of moving the unique pointer, you might actually want to just get() the raw pointer and pass that somewhere else? Raw pointers are still just fine, as long as you use them for non-owning purposes.

2 Likes

Maybe you’ll like my alternative instead:

Advantages:

  1. less typing:
EA::OwningPointer<Button> widget;

//creates a new Button:
widget.create();

//creates a new TextButton
widget.create<TextButton>();

void funcThatTakesRawPointer(Button* button);

//no .get() calls are needed:
funcThatTakesRawPointer(widget);
  1. It implements the ‘clone()’ pattern so if your derived types have that, you can copy a polymorphic container without lots of boilerplate.
1 Like

std::unique_ptr is more typing, but juce::ScopedPointer is broken. I think the worst was that it had operator=.

Meaning you’d do something like this:

auto foo = thing.that.is.complicated.foo;
foo->fwarble();

Oops, now you’ve deleted your object.

1 Like

This is what gets my goat:

void setSomething(unique_ptr<Component> b)
{
   b->doSomething();
   container.push_back(std::move(b));

  /// many lines of code

  if (someRareCondition())
   b->doSomethingElse();
}

Foot. Shot.

Yeah but I just never made that mistake in 10 years :wink:

If in doubt, I always use a shared_ptr. Not sure if that is best practice, but works for me :slight_smile:

1 Like

based

1 Like

emphasized text

I’m confused about that example.
So, you have a Component that get created somewhere, then you move it into the vector?

That kind of ownership structure confuses me.
Instead, I would create the object in place, using a factory method if there’s logic involved:

std::vector<std::unique_ptr<Component>> container;

container.emplace_back(createCompFactory());
auto& added = *container.back();
added.doSomething();

if (added.someRareCondition)
    added.doSomethingElse();
2 Likes

You mean you never make an object and then give it to another class?

It’s sort of the thing that say JUCE Viewport does, or a million other bits of the library. We do it all the time, especially in UI code where we want some reusable code to be agnostic about whatever component we pass to it is…

Yes, I think passing ownership is usually something you want to avoid in modern C++. Passing a reference/raw non owning pointer should be just fine.

ViewPort, IMO, should only take the Component it manages by reference/pointer and never delete it later, and the fact it does take ownership is for legacy reasons.

Alternatively if you want to create the component in place to be owned by the class, you can create it directly by doing something like:

class ManagesComponent
{
public:
    template <typename Derived, typename... Args>
    void create(Args&&... args)
    {
        comp = std::make_unique<Derived>(std::forward<Args>(args)...);
    }
private:
    std::unique_ptr<Component> comp;
};

And then never expose the creation structure or let the user do something risky like moving a unique_ptr and figuring out who owns it in two pieces of code.

2 Likes

Your example exactly highlights one of the benefits of unique_ptr. In many lines of code can you be sure b hasn’t be deleted from the container?

The semantics are correct here, if you’ve transferred ownership to something else, it’s that something else’s responsibly to give you access to it. Use container.back()?

4 Likes

This is what leads to bad practices like using shared_ptr all over the place IME.

Moving ownership is something you should try to limit as much as possible, but there are times when one object needs to hand off something it owned to another - that’s why we have move semantics after all?

6 Likes

I just don’t like it.

I don’t like change* :wink:

*Except for (if (auto x = something(); x != 12)) … that shit in if statements is wicked :slight_smile:

Depends on your container whether this is free or not probably? And I think it makes my code look ugly :slight_smile:

I mean really I think my issue is that just don’t like having a new category of error to worry about. And I’d trained myself to avoid all the pitfalls of the previous way without really having to think. And thinking is a lot of effort.

Tangentially - Ever wondered why thinking is a lot of effort? It’s a bit weird, it’s not like it uses loads more calories, so why does thinking hard feel like a strain… curious bit of evolution that!

Always use a shared_ptr? That’s just terrifying :slight_smile: Although many programs in other languages do just that!

1 Like

This is really bad advise. Not only is there an additional cost involved with shared_ptr, it completely tramples over all the ownership semantics that unique_ptr tries to enforce for you, for good reason. Very easy to get cyclic references if you’re always using shared_ptr…

10 Likes

BTW, I think a right way to enforce correct usage when moving a unique_ptr into a function is by using rvalue reference:

void takesPointer(std::unique_ptr<Object>&& obj)
{
    obj->doSomething();
}

int main()
{
    auto obj = std::make_unique<Object>();
    
    //Won't compile:
    takesPointer(obj);
    
    //Compiles
    takesPointer(std::move(obj));
}