class A {
private:
unsigned int nElements;
Object* obj;
public:
A() : nElements(0), obj(NULL) {}
~A() {
delete[] obj;
obj = NULL;
}
void createArray(unsigned int n) {
nElements = n;
obj = new Object[nElements];
for (unsigned int i = 0; i < nElements; i++) {
obj.initialize();
}
}
A(const A& original) {
nElements = original.nElements;
obj = new Object[original.nElements];
std::copy(original.obj, original.obj + original.nElements, obj);
}
A& operator=(const A& original) {
nElements = original.nElements;
obj = new Object[original.nElements];
std::copy(original.obj, original.obj + original.nElements, obj);
return *this;
}
};
Copy Constructors and Assignment Operators
How do I properly write a copy constructor and overload the assignment operator for my class? I've shown my attempt, it compiles but I just want to make sure it does what I'm expecting. Thanks for looking.
Edit: changed source code.
[Edited by - X Abstract X on March 28, 2010 5:34:13 PM]
No, your copy constructor and assignment operator only create one element no matter how many elements the other object contains. Also, your assignment operator leaks currently allocated memory. Is there any reason you aren't using std::vector?
Quote:Original post by SiCrane
No, your copy constructor and assignment operator only create one element no matter how many elements the other object contains. Also, your assignment operator leaks currently allocated memory. Is there any reason you aren't using std::vector?
Actually yes, I can't use std::vector for this. Thanks for pointing the flaws in my code. I've modified my source now and I believe this should work?
Your assignment operator doesn't check for the case where you might do something like this:
And like SiCrane said, your assignment operator will leak memory. Think about what happens to the previously stored data..
Bah I'll just make it easy for you:
EDIT: SiCrane makes me cry inside..
[Edited by - Wavarian on March 28, 2010 6:56:17 PM]
A a;...a = a; // Oops..
And like SiCrane said, your assignment operator will leak memory. Think about what happens to the previously stored data..
Bah I'll just make it easy for you:
const A& operator=(const A& original){ if(&original != this) // Safeguard for 'a = a' scenario { // Free existing data first! delete[] obj; // Reset object state obj = 0; nElements = 0; try { obj = new Object[original.nElements]; std::copy(original.obj, original.obj + original.nElements, obj); nElements = original.nElements; } catch(...) { delete[] obj; obj = 0; throw; } } return *this;}
EDIT: SiCrane makes me cry inside..
[Edited by - Wavarian on March 28, 2010 6:56:17 PM]
No, your assignment operator still leaks currently allocated memory. Additionally your copy constructor isn't exception safe; if std::copy throws an exception, you'll leak the allocated memory.
Edit: Wavarian's assignment operator isn't exception safe either. If the new[] expression throws, the object will be left in an invalid state.
Edit: Wavarian's assignment operator isn't exception safe either. If the new[] expression throws, the object will be left in an invalid state.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement