Task::Task(const Task& t){
*this = t;
}
Task& Task::operator=(const Task& t){
task_from = t.task_from;
task_to = t.task_to;
}
C++ code is it ok
Author
Just a couple of quick queries. Is it ok to overload the assignment operator (=) and then use it in a copy constructor like this.
also, in the operator assignment why is it that scope isn't an issue. I'm using g++ (cygwin/Eclipse) and it doesn't seem to mind me doing this.
When both task_from and task_to are private. Does it have something to do with it being the same class?
For your first question- I generally write out assignment operators and copy constructors separately but I'm not sure if there are any drawbacks either way so I'll let someone else field that.
For your second question- yes, since it is the same class it is allowed to access it's own and other instances of the same class' private members from within its own member functions
For your second question- yes, since it is the same class it is allowed to access it's own and other instances of the same class' private members from within its own member functions
For the first, I'm almost positive that that is just fine. I don't remember if I ever used an overloaded = in constructor, but it should work.
For the second, since = is a member function, it can change those two private variables. And the way it knows which task_from or task_to is which, is that, a "this" dereference is assumed when not shown.
i.e. THIS:
task_from = t.task_from;
task_to = t.task_to;
is equivalent to THIS:
this->task_from = t.task_from;
this->task_to = t.task_to;
(or THIS too... I'm just playing with syntax now...):
(*this).task_from = t.task_from;
(*this).task_to = t.task_to;
For the second, since = is a member function, it can change those two private variables. And the way it knows which task_from or task_to is which, is that, a "this" dereference is assumed when not shown.
i.e. THIS:
task_from = t.task_from;
task_to = t.task_to;
is equivalent to THIS:
this->task_from = t.task_from;
this->task_to = t.task_to;
(or THIS too... I'm just playing with syntax now...):
(*this).task_from = t.task_from;
(*this).task_to = t.task_to;
It is ok, but it is likely that if you need a custom copy constructor and assignment operator, you can't implement the copy constructor using the assignment operator. Generally, a custom assignment operator has to release some resource or deallocate some memory. Doing that might cause a problem in a copy constructor where there is no resource to release or memory to deallocate. Here is a simple example:
}
class foo { foo() { p = new int; } // assignment operator foo & operator=( foo & rhs ) { if ( this != &rhs ) { delete p; p = new int( *rhs.p ); } return *this; } // copy constructor foo( foo const & rhs ) { *this = rhs; // major problem: this->p is uninitialized so delete p will barf. } }; }
It's okay, the STL has you covered:
$ g++ -ggdb3 -Wall -Wextra -Werror -ansi -pedantic test.cpp -o test
$ valgrind ./test
<snip>
new 0x1bb36028
new 0x1bb36060
new 0x1bb36098 (from 0x1bb36028)
new 0x1bb360d0 (from 0x1bb36098)
del 0x1bb36060
new 0x1bb36108 (from 0x1bb360d0)
del 0x1bb36028
del 0x1bb36098
del 0x1bb360d0
del 0x1bb36108
==21675==
==21675== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 21 from 1)
==21675== malloc/free: in use at exit: 0 bytes in 0 blocks.
==21675== malloc/free: 5 allocs, 5 frees, 5 bytes allocated.
==21675== For counts of detected errors, rerun with: -v
==21675== No malloc'd blocks -- no leaks are possible.
#include <memory>#include <iostream>struct MyObject{ MyObject() { std::cout << "new " << this << std::endl; } MyObject( MyObject const & ref ) { std::cout << "new " << this << " (from " << &ref << ")" << std::endl; } ~MyObject() { std::cout << "del " << this << std::endl; }};struct foo{ typedef std::auto_ptr<MyObject> MyObjectPointer; MyObjectPointer p; foo(): p( new MyObject ) {} foo & operator=( foo const & rhs ) { if ( this != &rhs ) p = MyObjectPointer(new MyObject( *rhs.p )); return *this; } foo( foo const & rhs ) { *this = rhs; }};int main(){ foo a, b, c = a; a = b = c;}$ g++ -ggdb3 -Wall -Wextra -Werror -ansi -pedantic test.cpp -o test
$ valgrind ./test
<snip>
new 0x1bb36028
new 0x1bb36060
new 0x1bb36098 (from 0x1bb36028)
new 0x1bb360d0 (from 0x1bb36098)
del 0x1bb36060
new 0x1bb36108 (from 0x1bb360d0)
del 0x1bb36028
del 0x1bb36098
del 0x1bb360d0
del 0x1bb36108
==21675==
==21675== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 21 from 1)
==21675== malloc/free: in use at exit: 0 bytes in 0 blocks.
==21675== malloc/free: 5 allocs, 5 frees, 5 bytes allocated.
==21675== For counts of detected errors, rerun with: -v
==21675== No malloc'd blocks -- no leaks are possible.
Quote:Original post by willow01I may work in many cases, but is not a totally safe practice. The reason is that in an assignment operation, the left hand side has already been constructed, and is merely being overwritten / replaced.
Just a couple of quick queries. Is it ok to overload the assignment operator (=) and then use it in a copy constructor like this.
*** Source Snippet Removed ***
also, in the operator assignment why is it that scope isn't an issue. I'm using g++ (cygwin/Eclipse) and it doesn't seem to mind me doing this.
*** Source Snippet Removed ***
When both task_from and task_to are private. Does it have something to do with it being the same class?
However in this case the left hand argument being passed in has not yet been initialised.
This can make trouble in cases such as where the assignment operator first checks if some field in both objects are equal, before copying, for instance.
In many cases it will be fine. People can and do use it, without problems in many cases. However you have now been warned of the potential dangers of doing so.
Edit: John Bolton appears to have already demonstrated exactly this, and very nicely I might add - DOH![smile]
Quote:Original post by willow01
Is it ok to overload the assignment operator (=) and then use it in a copy constructor like this.
No. Just don't do it. There are many and varied, and sometimes subtle, reasons why it won't always work, or will work but not the way you expect. For a book-length treatment of the topic, try Sutter's Exceptional C++.
What you might want to do is implement your assignment operator in terms of the copy consructor. If you have a member function void swap() nothrow(), what you've done is provided the strong exception guarantee, and all your object-copying smarts is in a single function, the copy constructor (which will throw an exception if the class invariants are not met).
class C{public: C(const C& rhs); C& operator=(const C& rhs) { C tmp(rhs); swap(tmp); return *this; }protected: void swap(const C& rhs) nothrow();};This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
