1

Please note that with the words "object" and "move" in the title, I do not mean the C++-specific concepts of what an object is and what it means to move an object.

I have a local variable of a pretty simple struct type where I accumulate some values read from a config file.

struct Foo
{
    std::string name;
    float someValue;
    int someStuff;
};

// This stores the current settings
std::shared_ptr<Foo> currentFooSetting;

void readSettings()
{
    Foo f;
    f.name = ...;
    f.someValue = ...;
    f.someStuff = ...;
    if (everythingIsOk)
    {
        //TODO:
        //Delete the old object pointed to by currentFooSetting (if it's not pointed to by any other shared_ptr, of course)
        //Allocate some new memory to put f into... or maybe reuse *currentFooSetting if it's now free idc.
        //Copy f to the new memory location
        //
    }
}

If everything is OK, I want to move f onto the heap and have currentFooSetting point to that. How do I do that?

It seems I could do this:

std::shared_ptr<Foo> newFooSetting = std::make_shared<Foo>();
*newFooSetting = f;
currentFooSetting = newFooSetting;

(Using a local variable to better emphasize the separation between the allocation, copying, and replacing.) But even after reading through https://en.cppreference.com/w/cpp/memory/shared_ptr and https://en.cppreference.com/w/cpp/memory/shared_ptr/make_shared I have no idea whether that's "the way" of doing this.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
Niko O
  • 406
  • 3
  • 15
  • Implement a proper set of move assignment operator and move constructor for `Foo`, and `currentFooSetting.value() = std::move(f);` – JohnFilleau Mar 14 '22 at 15:05
  • Can you verify if you expect `currentFooSetting` to ever by empty when you call `readSettings`? – JohnFilleau Mar 14 '22 at 15:07
  • 1
    So, if someone else has a `shared_ptr` to the current settings, what should happen then? – Ted Lyngmo Mar 14 '22 at 15:08
  • 2
    `currentFooSetting = std::make_shared(std::move(f));` would be simpler than your last idea. It would also _move_ the content instead of copying it. If you want to assign to the current settings, then just do `*currentFooSetting = std::move(f);` – Ted Lyngmo Mar 14 '22 at 15:09
  • Why not always creating a new object inside a shared_ptr? Most times everything could be okay and in the other cases the shared_ptr just gets deallocated again as soon as it goes out of scope? – Sebastian Mar 14 '22 at 15:25
  • @JohnFilleau `currentFooSettings` is initialized to point to an object with default settings. `readSettings` is only called afterwards, so from its point of view, `currentFooSettings` should never be empty. – Niko O Mar 15 '22 at 13:29
  • @TedLyngmo It doesn't particularly matter, whether other pointers see that change. In my example, they don't, but that's not a requirement. – Niko O Mar 15 '22 at 13:32
  • @Sebastian Because I find it kinda silly to allocate before knowing whether I need the allocation. Not a big deal here, but knowing how to handle this simple case will hopefully help me in the future when more complicated cases arise. – Niko O Mar 15 '22 at 13:34
  • @NikoO Ok, I added a version supporting both. – Ted Lyngmo Mar 15 '22 at 14:30
  • Okay, but OTOH you have an additional move. Allocating local data (stack and/or registers) is cheap. But if the allocation is needed in 99.9% anyway. Reasons for both ways. – Sebastian Mar 16 '22 at 02:28

3 Answers3

3

How about

if (currentFooSetting) {
    *currentFooSetting = f;
} else {
    currentFooSetting = std::make_shared<Foo>(f);
}

this sort-of ensures that you have just one shared pointer, and once it is created, its value is changed on update. Alternately, if existing holders-of-the-shared-pointer should keep their values, just assign a new one:

currentFooSetting = std::make_shared<Foo>(f);

This will "fork" the views of current settings -- holders-of-a-shared-pointer keep the old one with old values, but functions that newly share the pointer get the new value.

Which makes more sense depends on your design (which is what Ted Lyngmo asks in a comment). Both code snippets assume your Foo class has suitable (copy) constructors.

2

It can be done more simply than the attempt in the question:

if (everythingIsOk) {
    currentFooSetting = std::make_shared<Foo>(std::move(f));
}

std::make_shared will allocate dynamic memory and the new object will be initialised by move from f. We could copy too, but in this case the local f is about to go out of scope, so it is safe to move from it.

Assigning currentFooSetting will cause it to destroy the previously pointed object (if any, and if there are no other owners), and currentFooSetting becomes the owner of the newly created object.

eerorika
  • 232,697
  • 12
  • 197
  • 326
  • Is there any benefit in using `std::move` here? Is it sufficient/correct/better/? to simply use `std::make_shared(f);`? – Niko O Mar 15 '22 at 14:55
  • @NikoO Moving is potentially much faster compared to copying. When moving is sufficient, it's better to move than to copy. – eerorika Mar 15 '22 at 15:08
0

Here's a readSettings function template supporting both move assigning to an existing Foo or unconditionally move constructing a new Foo (if everythingIsOk is true):

template<bool Reassign = true>
void readSettings() {
    Foo f;
    
    // ...

    if (everythingIsOk) {
        if constexpr (Reassign) {
            // move assign if you've already createad a shared Foo
            if(currentFooSetting) *currentFooSetting = std::move(f);
            // or move construct a new Foo
            else currentFooSetting = std::make_shared<Foo>(std::move(f));
        } else {
            // always move construct a new Foo
            currentFooSetting = std::make_shared<Foo>(std::move(f));
        }
    }
}

Then either use readSettings()/readSettings<true>() to move assign to an existing Foo or readSettings<false>() to always move construct a new Foo.

Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108