Reputation: 18625
I have this problem, there is a function foo()
as follows,
vector<ClassA> vec;
void foo()
{
ClassA a; //inside foo, a ClassA object will be created
a._ptr = new char[10];
vec.push_back(a); //and this newly created ClassA object should be put into vec for later use
}
And AFAIK, vec
will invoke ClassA
's copy-ctor to make a copy of the newly created object a
, and here is the problem. If I define ClassA
's copy-ctor the usual way,
ClassA::ClassA(const ClassA &ra) : _ptr(0)
{
_ptr = ra._ptr;
}
then object a
and its copy (created by vec) will have pointers _ptr
pointing to the same area, when foo
finishes, a
will call the destructor to release _ptr
, then a
's copy in vec
will be a dangling pointer, right? Due to this problem, I want to implement ClassA
's copy-ctor this way,
ClassA::ClassA(ClassA &ra) : _ptr(0) //take non-const reference as parameter
{
std::swap(_ptr, a._ptr);
}
Is my implementation ok? Or any other way can help accomplish the job?
Upvotes: 9
Views: 10716
Reputation: 3980
You should not copy the pointer, you should copy the context that the pointer is pointing to. You should also initialize the class by telling it how many elements you want, instead of allocating it by accessing a public pointer.
Since you want to copy the object, not move it, you should allocate resources in the new object when copying.
class A {
int* p_;
int size_;
public:
A(int size)
: p_(new int[size]()),
size_(size) {
}
A(const A &a)
: p_(new int[a.size_]),
size_(a.size_) {
std::copy(a.p_, a.p_ + a.size_, p_);
}
...
};
int main () {
A a(10);
vec.push_back(a);
}
However, if you know that the object you will copy isn't used after it's copied, you could move it's resources instead.
Upvotes: 2
Reputation: 8988
Additional comment:
Avoid these kind of problems creating the object with new
and storing pointers to the object in the vector.
Upvotes: 0
Reputation: 477710
To answer your titular question: Yes, any constructor for a class T
that has one mandatory argument of type T &
or T const &
(it may also have further, defaulted arguments) is a copy constructor. In C++11, there's also a move constructor which requires one argument of type T &&
.
Having a non-constant copy constructor that actually mutates the argument gives your class very unusual semantics (usually "transfer semantics") and should be extensively documented; it also prevents you from copying something constant (obviously). The old std::auto_ptr<T>
does exactly that.
If at all possible, the new C++11-style mutable rvalue references and move constructors provide a far better solution for the problem of "moving" resources around when they're no longer needed in the original object. This is because an rvalue reference is a reference to a mutable object, but it can only bind to "safe" expressions such as temporaries or things that you have explicitly cast (via std::move
) and thus marked as disposable.
Upvotes: 13
Reputation: 2031
The problem with your implementation is that you will not be able to pass temporary objects as arguments for this copy-ctor (temporaries are always const
). Like already mentioned the best solution would be to move to c++11 and use move semantics. If it is not possible shared_array
can be an alternative.
Upvotes: 1
Reputation: 13611
C++11 introduced move constructors for this exact purpose:
ClassA::ClassA(ClassA&& ra)
: _ptr(ra._ptr)
{
ra._ptr = nullptr;
}
Alternatively you can declare _ptr
to be a shared pointer:
std::shared_ptr<char[]> _ptr;
and then default denerated copy constructor will do just fine.
Upvotes: 4