4

For example:

boost::shared_ptr<int> test() {
    boost::shared_ptr<int> x(new int(3));
    return x;
}

void function() {
    int y = *test();
    ...
}

Is it also a bad idea to use shared_ptr to avoid copying the whole object? Like a vector of matrices/images for example.

chib
  • 43
  • 2

3 Answers3

4

In the general case, no. Your example copies the contents of the shared_ptr, and then the original value is deleted.

Now, the bigger issue here is that it's fantastically inefficient to do a dynamic memory allocation for an int, but I'm assuming you're not doing that in real code. :)

Billy ONeal
  • 104,103
  • 58
  • 317
  • 552
  • 2
    If we're talking about efficiency, it would also be more efficient to use `boost::make_shared` rather than to use `new`. :) – Sven Aug 18 '11 at 05:13
  • @Sven: That's true. (Though not really related to the question) – Billy ONeal Aug 18 '11 at 05:16
  • @Billy I'll ask again since this one is getting more upvotes: if it was an object whose operator= was only making a shallow copy, would this be a problem? – chib Aug 18 '11 at 05:16
  • @chib: If you were relying on deep copy semantics, of course that'd be a problem. :) – Billy ONeal Aug 18 '11 at 05:18
  • @Billy If I'm only using it inside a function, would that still be a problem? Or does the object only get destructed after the function ends? – chib Aug 18 '11 at 05:20
  • 2
    @chib: Here's what's going on. You are getting back a temporary `shared_ptr`, then copying it's contents to a completely different `int` (or whatever your complex object is). Your original object (with the derived semantics) is deleted at the end of the statement. Even if it's not deleted at the end of the statement, you're going to be constantly interacting with a shallow copy, so it doesn't much matter. – Billy ONeal Aug 18 '11 at 05:22
  • @Billy The thing I'm worried about is having the object survive enough to be used, but since it's only being used inside function(), I figure I wouldn't need yet another shared_ptr. I guess the right thing to do is to just fill the code with shared_ptrs. Thanks anyway. – chib Aug 18 '11 at 05:25
  • @chib: The last `shared_ptr` will be destroyed when the function exits, which will cause the object contained within to be `delete`d. – Billy ONeal Aug 18 '11 at 05:26
  • @chib: *the right thing to do is to just fill the code with `shared_ptr`s* or alternatively drop them from where they don't make sense. Either you want your own copy of the object or the object is shared, that is a property of the design. If the object is shared, then `shared_ptr` will help maintain the *shared* ownership, if it is not to be shared *always* you should not return `shared_ptr` from the function in the first place. You could return by value, or if you need dynamic allocation, return an `unique_ptr`. – David Rodríguez - dribeas Aug 18 '11 at 08:15
  • 2
    The `unique_ptr` is a deeper advice than most people notice, it buys the user *freedom* to decide how to manage the object: it can be held by a single owner by maintaining the `unique_ptr` or the receiving end can decide to *share* ownership (pass it to other parts of the code) by *moving* the object from the `unique_ptr` to a `shared_ptr` (interestingly, even `auto_ptr` is better than `shared_ptr` in this respect). Then again, if the function is also *sharing* the ownership with the caller, then `shared_ptr` is the way to go, and you should maintain your side of the agreement. – David Rodríguez - dribeas Aug 18 '11 at 08:17
2

In your example, that's fine, since you're making a copy of the int.

If you get the int as a reference, then after that line, it would be a dangling reference, since the shared pointer would go out of scope, deleting its target.

Is it also a bad idea to use shared_ptr to avoid copying the whole object? Like a vector of matrices/images for example.

Using shared_ptr will avoid copying just as using a naked pointer will avoid copying - decide whether you want to avoid copying (first), and then choose which sort of pointer you should use.

For a vector of matrices or images, you may want to use a std::vector of boost::shared_ptr, or a boost::ptr_vector, or some other container that makes the memory management easy for you.

Jesse Beder
  • 33,081
  • 21
  • 109
  • 146
  • What if it was a complex object instead? I mean, one whose operator= only made a shallow copy. – chib Aug 18 '11 at 05:10
  • @chib: Then it should be fine; just watch out for the slicing problem. (Though still, fantastically inefficient) (Jesse, +1) – Billy ONeal Aug 18 '11 at 05:11
  • @Billy Jesse Beder said it's only ok because I'm copying the int. So if it's an object with operator= doing a shallow copy, wouldn't that be a problem? – chib Aug 18 '11 at 05:13
  • @chib: It's basically okay, though it would invoke the object's copy ctor. However, note that it's not safe for polymorphic objects: if the function returns a `shared_ptr` that was created with an instance of a derived class instead, doing `Base b = *test();` would slice the object. – Sven Aug 18 '11 at 05:17
  • It's true that `shared_ptr` will avoid copying like a naked pointer, but I think the point of that would be to avoid manual memory deallocation, letting `shared_ptr` do it for you when the objects go out of scope, no? – Seth Carnegie Aug 18 '11 at 05:18
  • @Seth: I don't see how that's related to the question or answer here at all. I don't think this is about avoiding the copy -- memory allocation is almost certainly more expensive than just doing the copy. – Billy ONeal Aug 18 '11 at 05:19
  • Ah, I got the impression he was returning `shared_ptr`s so he could return objects neither by value nor by "out parameter reference" but not have the user have to worry about manual deallocation. I've never been wrong before of course though. – Seth Carnegie Aug 18 '11 at 05:22
1

I would say that yes, it is a bad idea.

If you're using a pointer there are 2 reasons. 1. Your object might be null, or 2. you have a big object that you don't want to copy.

It's rarely a good idea to use the value directly since you don't know if it's null or not.

Billy ONeal
  • 104,103
  • 58
  • 317
  • 552
Flame
  • 2,166
  • 2
  • 20
  • 40
  • -1 - why? (If you edit to add a why I'll remove the downvote) – Billy ONeal Aug 18 '11 at 05:11
  • 1
    Unless the function is documented as being able to return a `nullptr`, it damn well better not return `nullptr`. I completely disagree with you, but I'll remove the -1 because I said I would. – Billy ONeal Aug 18 '11 at 05:16
  • Related: http://stackoverflow.com/questions/4390007/in-either-c-or-c-should-i-check-pointer-parameters-for-null – Billy ONeal Aug 18 '11 at 05:18
  • I agree that it would be bad form. But you might be dealing with library code that has these design flaws. It's something to watch out for. – Flame Aug 18 '11 at 05:19
  • Then the bad idea was in the poorly written library, not in your code. (And it's better to show such cases immediately with a crash than to hide them and possibly ship them in production code) – Billy ONeal Aug 18 '11 at 05:20