2

How do I correctly use std::unique_ptr and std::weak_ptr in this case?

struct B;
struct A
{
    B* b;
    float f;

    A(float f, B* parent)
        : f(f), b(parent)
    {}
};

struct B
{
    A a;

    B(float f)
        : a(f, this)
    {}
};

I want to create a B with auto anB = std::unique_ptr<B>(new B(3.0f)). My guess would be that A should have a std::weak_ptr<B> b instead of the raw pointer, but to create the std::weak_ptr I need a std::shared_ptr of B...
Should A just keep its raw pointer in this case?

sro5h
  • 322
  • 2
  • 12
  • Consider using `auto anB = std::make_unique(3.0f);`. If all `A`s always point to the `B` that owns it then you can use a raw pointer without any trouble since it will always be valid (make sure you get copy and move semantics right). Otherwise you might want to just use `std::shared_ptr`s. – nwp Apr 12 '16 at 14:05
  • Im using msvc 2012 right now(there's no `std::make_unique` but I'll keep it in mind when I switch back to 2013 again :) And yes the `A`s lways Point to their owner – sro5h Apr 12 '16 at 14:07
  • @nwp: Note that `std::make_unique` is c++14. (even if it can be written in c++11). – Jarod42 Apr 12 '16 at 14:07
  • Without `std::make_unique` I would [consider writing my own](http://stackoverflow.com/questions/12547983). – nwp Apr 12 '16 at 14:11
  • I believe it's intentional. unique_ptr should have only one instance of it tracking the contained ptr. weak_ptr is there, among other reasons, so that you can create a second instance of a shared_ptr tracking the same contained object with the same counter as the original shared_ptr (otherwise you might have 2 shared_ptrs each with its own counter tracking the same object ptr and it would result in double deallocation). If you have a weak_ptr, you can lock() it. That would break the uniqueness of the unique ptr. – Dmitry Rubanovich Apr 12 '16 at 14:20
  • STOP. This is poor design. A should not have to hold on to a reference of B. It is entirely owned by B. If B wants to communicate information to it during method calls, it may do so by passing an argument. – Richard Hodges Apr 12 '16 at 14:29
  • In my particular case `B` has two `A`s. Furthermore I have two `std::vector`s, one has the `std::unique_ptr`s to the `B`s and the other the *sorted* `A`s. And I Need to find the corresponding `B` to an `A` when iterating through the sorted `std::vector` – sro5h Apr 12 '16 at 14:36
  • Then change it so the sorted vector is a vector of std::reference_wrapper, sorted with a predicate which examines B's A. The vector is simply an index. A is an attribute, B is the object. – Richard Hodges Apr 12 '16 at 15:02
  • But I sort the `A`s independently from its owning `B`. (Btw I said I have *two* `A`s for each `B`) What I want to say by that is: The *two* `A`s for each `B` dont have to be one after the other in the sorted `std::vector`, so I cant just create a sorted `std::vector` of `B`s. I Need to sort by `A::f` – sro5h Apr 12 '16 at 15:06
  • @sro5h, in the design you are describing, what do you expect to happen when one of B's gets deallocated? The A will go away with it. But A's reference (or pointer) will remain in your sorted vector. As long as you are aware of this and your design takes it into account, you probably want to use plain references to containing B's inside of A's (because for each A, the B instance is fixed). – Dmitry Rubanovich Apr 12 '16 at 18:28
  • Yes, I know I need to be careful with the sorted vector, for now I know it will get destroyed first. I'm pretty sure you cant use plain references in a vector right? Shall I do it like Richard Hodges does it in his answer with a reference_wrapper instead of a pointer? – sro5h Apr 12 '16 at 18:36
  • @sro5h, i meant use plain reference to B in A (instead of row pointer). But it still doesn't take care of the fact that the pointer inside the vector doesn't know when its object is destroyed. The problem with reference_wrapper is that it doesn't fix the vector issue, either. I think this maybe trying to eat your cake and have it, too. You want a unique_ptr, but you also want other references to it (just not through this pointer). You may be better off switching to shared_ptr and using weak_ptr in the vector (and sort with a predicate). – Dmitry Rubanovich Apr 12 '16 at 19:23

3 Answers3

4

The information provided is not enough information to solve the lifetime management problem.

You have provided some structure layouts. You have not described how the data in the structures is going to be used, nor anything much about their lifetimes.

Structure layouts does not determine lifetime, except a sane structure layout is going to be a function of the data lifetime problem.

What you have done is said "I have some red paint I want to paint a room with. What house should I buy?" Structure layout is useful to solve lifetime issues, and paint is useful to paint rooms in houses, but providing a structure layout does not tell you what smart pointers to use, and the color of house paint doesn't tell you what house to buy.

You need to think about what the lifetime of the various objects will be, and try to make that as simple as possible, and then use the smart pointers that make that management easy.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • Thank you, I think that greatly helped me. Each `A` is always created by a `B` and therefor I guess I should use a raw pointer in `A`, because the lifetime of `A` never excells the lifetime of ist owning `B` (I hope the conclusion is correct) – sro5h Apr 12 '16 at 14:18
  • @sro5h sure. Then enforce it -- make `A`'s ctor private, `B` a friend, block all copy-type operations (make private or delete). – Yakk - Adam Nevraumont Apr 12 '16 at 14:21
2

As A doesn't have ownership of B, a raw (non owning) pointer is ok. (but its lifetime should be longer than A).

So currently, you have problem with default copy constructor/assignment of B/A which may make A points to old B.

For the weak_ptr, indeed, B should go in a shared_ptr instead of unique_ptr, and to allow same signature, it should inherit from std::enable_share_from_this...

Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • I'm sorry but could you further explain your second sentence, I dont really understand what you mean :/ – sro5h Apr 12 '16 at 14:09
  • @sro5h if you copy one instance of `B` to another like `B newb = oldb;' `newb.a` will still point to `oldb` if you would not create proper copy/move ctor and assignment operator (or you may prohibit them). – Slava Apr 12 '16 at 14:15
0

From our conversation in the comments it seems to me that separating concerns might be in order.

An A is an attribute of a B. There needs to be a canonical store of Bs (that controls lifetime) and there needs to be an index (non-owning) of Bs ordered by some attribute of the B (in this case, one of the As).

This argues for two vectors (or some other appropriate container), one containing Bs and the other containing sorted references to Bs.

You can of course wrap the container and index into another object to provide encapsulation (I have not done this in the trivial example below):

#include <vector>
#include <algorithm>
#include <memory>
#include <functional>

// define the attribute
struct A
{
    float f;

    A(float f)
        : f(f)
    {}
};

// define the object    
struct B
{
    A a1;
    A a2;

    // no back-pointers
    B(float f, float f2)
        : a1(f)
          , a2(f2)
    {}
};


int main()
{
  using b_vec_type = std::vector<B>;

  // build the canonical store      
  b_vec_type bs = { B { 1, 2}, B { 5, 4 }, B { 3, 4 } };

  using a1_index_type = std::vector<std::reference_wrapper<B>>;

  // build the index
  a1_index_type index(bs.begin(), bs.end());

  // sort the index by the attribute we want      
  std::sort(index.begin(), index.end(), 
            [](const B& l, const B& r) {
              return l.a1.f < r.a1.f;
            });

  // now use the index:

  for (const auto& e : index)
  {
    // do something with the Bs, ordered by B::a1
    // e will be of type std::reference_wrapper<B>, which has
    // a conversion operator to B& already defined.
  }


  return 0;
}
Richard Hodges
  • 68,278
  • 7
  • 90
  • 142
  • My Goal for the sorted vector is to have a list of all existing `A`s sorted by their member f. I dont want a sorted list of `B`s sorted by ´a1.f`. I think we're both talking about slightly different things :/ – sro5h Apr 12 '16 at 15:20
  • @sro5h you can get to the A from the B. You will want to avoid getting to the B from the A, as the A is not a real object - it's an attribute. – Richard Hodges Apr 12 '16 at 15:29
  • I will have a look on my thought behind it again. Maybe you are right and what I want could be achieved without the pointer back to B – sro5h Apr 12 '16 at 16:09