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:
- 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.
- 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.
- How Can I write the code above using unique_ptr?
If copy-constructing B
is costly, then (smart)pointers are probably a good idea (redesigning the application logic might be another solution),
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,
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
}
};
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.