Passing object by unique_ptr or by value and how to implement

Question

I have a case that I am not sure if I should use unique_ptr or pass Object by values.

Say that I have class A which has a vector Of class B and Class C has a vector of class B as well. Every time I am adding a B Object to Vector in class C it should be Removed from vector of Class C and vice versa. When Object C is destroyed all the objects in B Vector class should be add to B vector in class A

class B {
public:
    B();
    virtual ~B();
};

class A {
    C & c;
    std::vector<B> bs;
public:
    A(C & c ): c(c){};
    virtual ~A();
    void add(B b){
        bs.push_back(b);
        c.remove(b);
    }
    void remove(B b){
        bs.erase(std::remove(bs.begin(), bs.end(), b), bs.end());
    }
};

class C {
public:
     A & a;
    std::vector<B> bs;
    C(A & a): a(a) {

    };
    virtual ~C(){
        for (B b : bs) {
                a.add(b);
                remove(b);
            }
    }

    void add(B b){
        bs.push_back(b);
        a.remove(b);
    }
    void remove(B b){
        bs.erase(std::remove(bs.begin(), bs.end(), b), bs.end());
    }
};

My questions:

1. Is it better to use pointers in this case? I should always have a unique Object for B! In other words if content of two b objects are different they are still different because different address in memory.
2. I want to write this code with help of smart_pointers in C++ 11! Which type is better shared_ptr or unique_ptr? A B object is never owned by two objects and it has always one owner so I guess unique_ptr is better but I am not sure.
3. How Can I write the code above using unique_ptr?

Answer 1

1. If copy-constructing B is costly, then (smart)pointers are probably a good idea (redesigning the application logic might be another solution),

2. If I understood correctly, a given B instance is always manipulated by a single owner (either A or C). std::unique_ptr is therefore a reasonable choice,

3. Try the following implementation. I haven't compiled it but I think you'll get the general idea :)

.

class B {
public:
    B();
    virtual ~B();
};

class A {
    C & c;
    std::vector<std::unique_ptr<B>> bs;
public:
    A(C & c ): c(c){};
    virtual ~A();
    // http://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/
    // (c) Passing unique_ptr by value means "sink."
    void add(std::unique_ptr<B> b){
        c.remove(b);               // release the poiner from the other container
        bs.emplace_back(b.get());  // emplace the pointer in the new one
        b.release();               // emplacement successful. release the pointer
    }
    // http://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/
    // (d) Passing unique_ptr by reference is for in/out unique_ptr parameters.
    void remove(std::unique_ptr<B>& b){
        // @todo check that ther returned pointer is != bs.end()
        std::find(bs.begin(), bs.end(), b)->release();             // first release the pointer
        bs.erase(std::remove(bs.begin(), bs.end(), b), bs.end());  // then destroy its owner
    }
};

class C {
public:
    A & a;
    std::vector<std::unique_ptr<B>> bs;
    C(A & a): a(a) {

    };
    virtual ~C(){
        for (auto&& b : bs) {
            a.add(b);
            // a is going to call this->remove()...
            // unless calling this->remove() from "a"
            // while this is being destroyed is Undefined Behavior (tm)
            // I'm not sure :)
        }
    }
    // http://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/
    // (c) Passing unique_ptr by value means "sink."
    void add(std::unique_ptr<B> b){
        c.remove(b);               // release the poiner from the other container
        bs.emplace_back(b.get());  // emplace the pointer in the new one
        b.release();               // emplacement successful. release the pointer
    }
    // http://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/
    // (d) Passing unique_ptr by reference is for in/out unique_ptr parameters.
    void remove(std::unique_ptr<B>& b){
        // @todo check that ther returned pointer is != bs.end()
        std::find(bs.begin(), bs.end(), b)->release();             // first release the pointer
        bs.erase(std::remove(bs.begin(), bs.end(), b), bs.end());  // then destroy its owner
    }
};

Answer 2

I would only use unique_ptr if you have to. You may prefer to make B a move-only type (like unique_ptr) to restrict ownership.

If B is expensive to move or it is not practical to prevent the copying of B then use unique_ptr but be aware that you are then paying for a dynamic memory allocation.

Here is how you could use a move-only B in an example inspired by your code. If you use unique_ptr instead it should work exactly the same:

struct B {
    B();
    B(B&&) = default;                 // Explicitly default the 
    B& operator=(B&&) = default;      // move functions.

    B(const B&) = delete;             // Delete copy functions - Not strictly 
    B& operator=(const B&) = delete;  // necessary but just to be explicit.
};

struct A {
    std::vector<B> bs;
    void add(B b){
        bs.push_back(std::move(b));
    }
    B remove(std::vector<B>::iterator itr){
        B tmp = std::move(*itr);
        bs.erase(itr);
        return tmp;
    }  
};

struct C {
    A& a;
    std::vector<B> bs;
    C(A& a) : a(a) {}

    ~C(){
        for (auto& b : bs) {
            a.add(std::move(b));
        }
    }  // bs will be deleted now anyway, no need to remove the dead objects

    void add(B b){
        bs.push_back(std::move(b));
    }
    B remove(std::vector<B>::iterator itr){
        auto tmp = std::move(*itr);
        bs.erase(itr);
        return tmp;
    }
};

int main() {
    A a;
    C c(a);
    a.add(B());
    auto tmp = a.remove(a.bs.begin());
    c.add(std::move(tmp));
}

Live demo.