make_shared inside a setter method behaves like creating a copy

Question

I'm trying to use smart pointers in my class, but, I don't know why, I can't make it work. Here is a simplified version of my class:

class Project {
public:
    std::shared_ptr<Part> getPart() const {
        return m_part;
    }
    void setPart(const Part &part) {
        m_part = std::make_shared<Part>(part);
    }

private:
    std::shared_ptr<Part> m_part;
};

The problem is if I pass a Part object (let's call it some_part) to the setPart() method and after making changes to some_part, the property m_part is unchanged, like if it points to a copy rather than to some_part itself. Why am I seeing this behaviour?

Is your goal to change root and have it reflect in Project::m_tree as well? — Stephen Newell, Mar 04 '18 at 21:08
Why? I thought that doing `const Part &tree` was for making a reference and not a copy. — Louis Etienne, Mar 04 '18 at 21:08
Yes. If I change the root during the execution of the program, m_root, as it points to it, should return root with its new values. — Louis Etienne, Mar 04 '18 at 21:09
@Wizix read this http://en.cppreference.com/w/cpp/memory/shared_ptr/make_shared — llllllllll, Mar 04 '18 at 21:17

einpoklum · Accepted Answer · 2018-03-04T21:39:22.550

2

The "problem" is with your use of make_shared. That utility function is not intended for wrapping plain pointers with shared pointers - it's intended for constructing a new object and returning a (shared) pointer to it.

But this is not really a problem, in the sense that you cannot transfer ownership of an object by passing const reference to it. So your class' interface is problematic to begin with - it doesn't fit shared ownership of a Part.

To do what you (seem to) want, you probably need your setter method to be something like:

void setPart(const std::shared_ptr<Part>& part);

so every instance on which setPart() is called gets the same shared pointer (or a copy thereof), and doesn't make its own.

Note: In some cases it may make sense to pass it by value:

void setPart(std::shared_ptr<Part> part);

edited Mar 04 '18 at 21:39

answered Mar 04 '18 at 21:14

einpoklum

118,144
57
340
684

Oh okay. So how should I do it? Pass a shared_pointer directly to the setter? I tried, but I couldn't make it works. – Louis Etienne Mar 04 '18 at 21:17
Thanks it worked! I think I understood how it works. – Louis Etienne Mar 04 '18 at 21:27
1

@Wizix: Wait, one last point... it's not entirely clear whether it's better to pass the shared pointer by value or as a const reference. See [this question](https://stackoverflow.com/questions/3310737/shared-ptr-by-reference-or-by-value). – einpoklum Mar 04 '18 at 21:29
1

@Wizix: And see my extra edit to your question, as you can inline methods (either in your actual code or at least here on StackOverflow, for shorter questions...) – einpoklum Mar 04 '18 at 21:42

make_shared inside a setter method behaves like creating a copy

1 Answers1