1

Background

With normal pointers, I can do something like the following

void conditional_reassign(MyClass* ptr)
{
    if (my_condition)
    {
        delete ptr;
        ptr = new MyClass(new_param);
    }
}

And I can pass in the pointer I want to change like following

MyClass* ptr = new MyClass(old_param);
conditional_reassign(ptr);

I wish to reimplement this with std::unique_ptr. Here's what I came up with

std::unique_ptr<MyClass> conditional_reassign2(std::unique_ptr<MyClass> ptr)
{
    if (my_condition)
    {
        ptr = std::make_unique<MyClass>(new_param);
    }
    return std::move(ptr);
}

And I would call it with the following

std::unique_ptr<MyClass> ptr = make_unique<MyClass>(old_param);
ptr = std::move(conditional_reassign2(std::move(ptr)));

Question

I'm not quite happy with the verbosity of the line

ptr = conditional_reassign2(std::move(ptr));

Is there a way to implement conditional_reassign2 so that I can call it in a manner similar to conditional_reassign(ptr)

Edit

I should note a major issue with

ptr = std::move(conditional_reassign2(std::move(ptr)));

is that it will destroy the original object ptr pointed to regardless of my_condition (see Why does reassigning a smart pointer to itself cause destruction?)

Community
  • 1
  • 1
Rufus
  • 5,111
  • 4
  • 28
  • 45
  • 5
    Apologies if I'm misreading your question, but `conditional_reassign` doesn't do what you appear to think it does. It does not change the caller's `ptr`. For that, you'd need to pass it by reference or make the function take a pointer to pointer. – NPE Apr 11 '19 at 09:39
  • The outer std::move is redundant. – eerorika Apr 11 '19 at 09:49
  • Why do you use same ptr for return value and as a parameter? – Dmitry Gordon Apr 11 '19 at 09:50
  • 3
    Your `conditional_reassign()` function (as pointed out) results in undefined behavior, and is a prime example of why you should convert your code to use `unique_ptr`. Your current bug is way more difficult to implement with `unique_ptr` :-) – Nikos C. Apr 11 '19 at 09:58

3 Answers3

3

Either you need to pass the pointer by reference

void conditional_reassign2(std::unique_ptr<MyClass>& ptr) {...}

std::unique_ptr<MyClass> myPtr;
conditional_reassign2(myPtr);

or return the pointer, which requires a single move

std::unique_ptr<MyClass> conditional_reassign2(std::unique_ptr<MyClass> ptr) {...}

std::unique_ptr<MyClass> myPtr;
myPtr = conditional_reassign2(std::move(myPtr));

Also you can return ptr directly from the function without explicitly calling move.

std::unique_ptr<MyClass> conditional_reassign2(std::unique_ptr<MyClass> ptr)
{
    if (my_condition)
        ptr = std::make_unique<MyClass>(new_param);
    return ptr;
}
1

You can define the conditional_reassign2() function to take the std::unique_ptr by reference instead of by value:

void conditional_reassign2(std::unique_ptr<MyClass>& ptr)
{
    if (my_condition)
    {
        ptr = std::make_unique<MyClass>(new_param);
    }
}

This way the function can directly modify the instance that is being passed, no need to transfer the ownership.

Assuming that ptr is an std::unique_ptr<MyClass>, then calling conditional_reassign2() would be in this case:

conditional_reassign2(ptr);
JFMR
  • 23,265
  • 4
  • 52
  • 76
0

Your first example does not do what you intend it to do. As ptr is passed by value, the caller's pointer will not be modified. So if my_condition is true, the caller has a pointer that points to a deleted object, and the address to which the newly created object is stored at is lost after the function returns.

Here is your first example fixed (the argument is now a reference to a pointer):

void conditional_reassign((MyClass*)& ptr)
{
    if (my_condition)
    {
        delete ptr;
        ptr = new MyClass(new_param);
    }
}

To use unique_ptr, you can also use references and not return anything. This way no need to deal with std::move

void conditional_reassign2(std::unique_ptr<MyClass>& ptr)
{
    if (my_condition)
    {
        ptr =  std::make_unique<MyClass>(new_param);
    }
}

You can call it like this:

std::unique_ptr<MyClass> ptr = make_unique<MyClass>(old_param);
conditional_reassign2(ptr);

Or you can use return/move semantics:

std::unique_ptr<MyClass> conditional_reassign(std::unique_ptr<MyClass> ptr)
{
    if (my_condition)
    {
        return std::make_unique<MyClass>(new_param);
    }

    return ptr;
}

And call it like this:

std::unique_ptr<MyClass> ptr = make_unique<MyClass>(old_param);
ptr = conditional_reassign2(std::move(ptr));
op414
  • 582
  • 5
  • 15