4 ms·

If you're going to pass it by const_ref, why not just pass the actual stored object by const_ref instead?

by sharth 10y ago

corysama 10y ago
Edit: Ignore this comment. I shouldn't talk so much about something I use so little. Somehow I skimmed through http://en.cppreference.com/w/cpp/memory/shared_ptr/shared_ptr http://en.cppreference.com/w/cpp/memory/shared_ptr/shared_pt... and thought that shared_ptr( const shared_ptr& r ); was missing. >> That's a good point. If the function will eventually lead to some object retaining a reference, then you should use a non-const shared_ptr ref. The assignment into the object will do the copy constructor and that needs a non-const ref. But, you don't need to be making temp object copies along the way. If the function will not lead to something retaining a reference, then you shouldn't be passing the reference retention object to it. Just pass a direct ref to the target object.
- tomjakubowski 10y ago
  > The assignment into the object will do the copy constructor and that needs a non-const ref. Hum? What assignment into the object? Where I'd typically see taking a `const std::shared_ptr<> &` to signal "retaining ownership" would be something like this: class Foo { public: void AppendChild(const std::shared_ptr<Foo> &x) { children_.emplace_back(x); } private: std::vector<std::shared_ptr<Foo>> children_; }; Why should Foo:;AppendChild's signature be changed to a non-const ref?
  - corysama 10y ago
    You are correct. I had a brain fart. Edited my comment.
  - millstone 10y ago
    It should be changed to pass by value, and then use std::move to move it into the children array. The reason is that if the argument is a temporary, you are making an unnecessary copy of that temporary, when you could just transfer ownership. This also gives callers the flexibility to std::move into your argument, transferring ownership. A good rule of thumb is, if you are going to unconditionally take ownership of an object, accept it by value.

If you're going to pass it by const_ref, why not just pass the actual stored object by const_ref instead?

by sharth 10y ago

If you're going to pass it by const_ref, why not just pass the actual stored object by const_ref instead?

Edit: Ignore this comment. I shouldn't talk so much about something I use so little. Somehow I skimmed through http://en.cppreference.com/w/cpp/memory/shared_ptr/shared_ptr http://en.cppreference.com/w/cpp/memory/shared_ptr/shared_pt... and thought that shared_ptr( const shared_ptr& r ); was missing. >> That's a good point. If the function will eventually lead to some object retaining a reference, then you should use a non-const shared_ptr ref. The assignment into the object will do the copy constructor and that needs a non-const ref. But, you don't need to be making temp object copies along the way. If the function will not lead to something retaining a reference, then you shouldn't be passing the reference retention object to it. Just pass a direct ref to the target object.

tomjakubowski 10y ago
> The assignment into the object will do the copy constructor and that needs a non-const ref. Hum? What assignment into the object? Where I'd typically see taking a `const std::shared_ptr<> &` to signal "retaining ownership" would be something like this: class Foo { public: void AppendChild(const std::shared_ptr<Foo> &x) { children_.emplace_back(x); } private: std::vector<std::shared_ptr<Foo>> children_; }; Why should Foo:;AppendChild's signature be changed to a non-const ref?
- corysama 10y ago
  You are correct. I had a brain fart. Edited my comment.
- millstone 10y ago
  It should be changed to pass by value, and then use std::move to move it into the children array. The reason is that if the argument is a temporary, you are making an unnecessary copy of that temporary, when you could just transfer ownership. This also gives callers the flexibility to std::move into your argument, transferring ownership. A good rule of thumb is, if you are going to unconditionally take ownership of an object, accept it by value.