4

I'm fairly new to smart pointer usage, and want to make sure I'm not introducing bad habits. I have an object A that is being created from a factory, and added to a queue. This queue is then read from another thread, and at this point I think it's perfectly valid to use a unique_ptr.

struct IInterface
{

}

class A : public IInterface
{

}

std::queue<std::unique_ptr<A>> queue;

However, while constructing A I realize I need a Class C. C takes a List of B's that it owns.

class A : public IInterface
{
    C c;
}

class B
{
}

class C
{
    List<unqiue_ptr<B>> b;
}

To me this still looks okay because we have ownership defined. However, I'm unsure of what pointer needs to be used when Class B now needs a pointer to IInterface, which A happens to implement. Do I make it a shared_ptr?

struct IInterface
{

}

class A : public IInterface
{
    C c;
}

std::queue<std::shared_ptr<A>> queue;



class B
{
    shared_ptr<IInterface> a;
}

class C
{
    List<unique_ptr<B>> b;
}

Did I choose the correct smart pointer, and did I analyze this correctly? I know this is super simple for you guys, but figured I would ask before I start doing things incorrectly.

Taztingo
  • 1,915
  • 2
  • 27
  • 42
  • 1
    Why does C store `unique_ptr` instead of `B`? – juanchopanza Mar 21 '18 at 19:58
  • Factory classes/functions should usually return `std::unique_prt`s. If that's all the caller needs it's perfect. If the caller needs shared ownership they can just store the return value in a `std::shared_ptr` and get just that. – Jesper Juhl Mar 21 '18 at 19:59
  • @NeilButterworth The example was geared towards smart pointers instead of public/private inheritance. You do make a good point so I'll update it for clarification. – Taztingo Mar 21 '18 at 20:00
  • 1
    See Herb Sutter's talk [*Leak-Freedom in C++... By Default*](https://www.youtube.com/watch?v=JfmTagWcqoE) – Justin Mar 21 '18 at 20:01
  • 4
    Use `std::unique_ptr` when you own something; raw pointer to observe something and `std::shared_ptr` when you need to re-design. – Richard Critten Mar 21 '18 at 20:02
  • @RichardCritten If I'm just observing wouldn't it be better to use a reference? – Taztingo Mar 21 '18 at 20:06
  • 3
    @Taztingo Depends on the circumstance. If you are storing something in an object, I would usually recommend pointers rather than references, as references can't be rebound, so you can't copy the object if you used them. – Justin Mar 21 '18 at 20:07
  • Okay cool these answers help a lot. Damn you guys are good! @Justin I'll take a look at the video you posted, thanks. – Taztingo Mar 21 '18 at 20:09
  • 1
    @Taztingo question was about pointers - references are also useful. – Richard Critten Mar 21 '18 at 20:09
  • Unrelated: `std::queue` should be pretty safe, but before building much infrastructure around pointers to library container contents, make sure you're familiar with the [Iterator Invalidation Rules](https://stackoverflow.com/questions/6438086/iterator-invalidation-rules). – user4581301 Mar 21 '18 at 20:17
  • 3
    It still isn't clear why you need any kind of pointer to `B` at all. – juanchopanza Mar 21 '18 at 20:24
  • @juanchopanza The pointers to B are there because C owns B, and are manually added to it's list. It was added to replicate the scenario I was facing, without having all my code. The question was more directed at the pointers for A. You did help me fix up some minor points that may have caused confusion, thanks for asking about it. – Taztingo Mar 22 '18 at 02:18
  • @user4581301: Even if `queue` moves its elements (which it might, given a non-default backing container), that wouldn’t invalidate pointers to the objects _pointed to_ by those elements. – Davis Herring Mar 22 '18 at 02:30
  • Yeah. Not sure what I was worried about. – user4581301 Mar 22 '18 at 05:06
  • @Taztingo That is no reason to have pointers of any type. Some crucial information is missing. Or you don't need pointers to B. – juanchopanza Mar 22 '18 at 06:00

1 Answers1

2

Leaving aside why C owns its B objects via a pointer, the question is not whether A is used via some base class. Indeed, a shared_ptr<Base> can be used to own an object of a derived class even if Base has a non-virtual destructor!

The real question is whether the lifetime of the B objects owned by a C (which is probably the same as the lifetime of the C) is (known to be) shorter than that of the A objects to which they refer. If it is, you can stick to unique_ptr<A> for owning the As and A* (or IInterface*) for using them. If not, you should consider using shared_ptr for both, although you should also consider other possibilities like destroying individual B objects sooner or transferring the ownership of the A to a place where it can outlive the observing references to it.

Davis Herring
  • 36,443
  • 4
  • 48
  • 76
  • Do you mean shorter than or equal to the lifetime, then use unique? My lifetime happens to be equal. – Taztingo Mar 22 '18 at 02:32
  • @Taztingo: It’s rare to have two objects be destroyed _simultaneously_: the only case that comes to mind is destroying an array of a type with no destructor. But if that can somehow happen, than equal would suffice. – Davis Herring Mar 22 '18 at 02:58
  • I guess in my case I have A, B, C all being generated at once, and A happens to be passed in. Once A is destroyed the others are no longer used. – Taztingo Mar 22 '18 at 03:02
  • @Taztingo: It is of course sufficient that `B::a` (for any object of type `B`) not be accessed (even to, say, compare it to `nullptr`!) after the referent has been destroyed. I wrote "shorter lifetime" as a design guideline for safety: if you have destroyed the `B`, you can't possibly access its pointer too late. – Davis Herring Mar 22 '18 at 03:35