constructor and reset method
Christopher
class A
{
int m_x;
int m_y;
public:
A()
:
m_x(0),
m_y(0)
{
}
~A(){}
Reset()
{
m_x = 0;
m_y = 0;
}
};
class B
{
int m_x;
int m_y;
public:
B()
:
m_x(0),
m_y(0)
{
}
~B(){}
Reset()
{
B();
}
};
Both seem to compile and work as expected, but as I've learned over
the years, just because it runs and shows you what you want to see,
doesn't mean it works.
Jeff Schwab
> Embarrassed to ask, but is there any functional difference between
[code like:]
class A {
int m_x;
int m_y;
public:
A() : m_x(0), m_y(0) { }
void Reset() {
m_x = 0;
m_y = 0;
}
};
class B {
int m_x;
int m_y;
public:
B(): m_x(0), m_y(0) { }
void Reset() { B(); }
};
> Both seem to compile and work as expected, but as I've learned over
> the years, just because it runs and shows you what you want to see,
> doesn't mean it works.
Smart man. :)
The method B::reset is not invoking the constructor on the right object.
It is only creating a temporary object of type B, and in no way
affecting the object on which Reset was called.
Your A::Reset approach is generally the right way to do this.
Alf P. Steinbach
B::Reset doesn't do anything.
What you have is a construction of a temporary.
That isn't a bad idea in and of itself (although it's probably not
what you intended), but it needs a little more support machinery:
class B
{
int myX;
int myY;
public:
B(): myX(0), myY(0) {}
reset() { *this = B(); }
};
Cheers, & hth.,
- Alf
--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
Alf P. Steinbach
Hm. Add a void.
Jeff Schwab
> * Christopher:
> reset() { *this = B(); }
I've never seen that before, but it does seem like a neat idea. :)
It seems like it would be (generally speaking) twice as slow as manual
member-by-member initialization, because of (a) the temporary object's
constructor, plus (b) the assignment operator.
jason.c...@gmail.com
news:13s9503...@corp.supernews.com...
> class B
> {
> int myX;
> int myY;
> public:
> B(): myX(0), myY(0) {}
>
> reset() { *this = B(); }
> };
Sometimes I go the other way around, as long as B is simple enough
that you can call Reset() on an unitialized B:
class B
{
int myX;
int myY;
public:
B() { Reset(); }
void Reset () {
myX = 0;
myY = 0;
}
};
But if you have to deallocate memory or something you still need to
initialize some things in the constructor:
class B
{
int *myInts;
public:
B() : myInts(NULL) { Reset(); }
void Reset() { // or something to this effect...
delete[] myInts;
myInts = NULL;
}
};
...and that all breaks down and turns pretty useless anyway once B
starts having members that are other complex objects (that you'd also
want to "Reset").
Jason
jason.c...@gmail.com
Just now I managed to get this working using placement new. Is there
anything bad that can happen with the following code (assuming the
constructor does not throw any exceptions):
class B {
public:
B () { }
~B () { }
void Reset () {
this->~B();
new (this) B();
}
};
I've done a good amount of testing on that just now and it seems to
work fine (even when B has other complex object members, etc.).
Assuming it's safe, it doesn't require you to implement an assignment
operator for complex B's, and doesn't create any temporaries. It seems
like it's just a different way of expressing the following, with the
addition of automatically calling member object destructors and
constructors:
class B {
public:
B () { Init(); }
~B () { Cleanup(); }
void Reset () {
Cleanup();
Init();
}
private:
// these do things:
void Init () { ... }
void Cleanup () { ... }
};
Jason
Jeff Schwab
The primary thing I would be concerned about is overwriting any
subobjects of B. If B has non-POD members, they may not take kindly to
be reconstructed in place, without any chance to delete resources they
were already holding.
For example: If B has a std::vector<int> member, the vector likely has
a pointer to some dynamically allocated memory. By using placement new,
rather than emptying the vector through its intended interface, you may
be overwriting the vector's pointer to its dynamically allocated memory,
such that the memory is leaked.
Alf P. Steinbach
> On Feb 26, 11:12 pm, Jeff Schwab <j...@schwabcenter.com> wrote:
>> Alf P. Steinbach wrote:
>>> * Christopher:
>>> reset() { *this = B(); }
>> I've never seen that before, but it does seem like a neat idea. :)
>>
>> It seems like it would be (generally speaking) twice as slow as manual
>> member-by-member initialization, because of (a) the temporary object's
>> constructor, plus (b) the assignment operator.
No. Thinking about that would be premature optimization. Actually a 'reset'
member function indicates of itself premature optimization, the idea of reusing
an object to avoid some imagined micro-inefficiency, thereby complicating the
design.
For optimization, always first measure.
> Just now I managed to get this working using placement new. Is there
> anything bad that can happen with the following code (assuming the
> constructor does not throw any exceptions):
>
> class B {
> public:
> B () { }
> ~B () { }
>
> void Reset () {
> this->~B();
> new (this) B();
> }
>
> };
>
> I've done a good amount of testing on that just now and it seems to
> work fine (even when B has other complex object members, etc.).
> Assuming it's safe,
It isn't safe, in general.
In particular it's problematic if anyone ever derives from B: then you're
constructing the wrong kind of object.
To reuse the constructor logic e.g. in assignment operator, instead add a swap()
member function.
> it doesn't require you to implement an assignment
> operator for complex B's,
Assignment operator or not has nothing to with the above issue.
> and doesn't create any temporaries.
Temporaries are not inefficient. They're not ungood. You should not focus on
avoiding temporaries.
Triple-DES
class B
{
public:
void Reset()
{
this->~B();
new (this) B();
}
virtual ~B() {}
};
class D : public B
{
// ...
};
int main()
{
B * b = new D; // Ok, *b is of type D
b->Reset(); // Oops, *b is of type B
}
DP
jason.c...@gmail.com
> For example: If B has a std::vector<int> member, the vector likely has
> a pointer to some dynamically allocated memory. By using placement new,
> rather than emptying the vector through its intended interface, you may
> be overwriting the vector's pointer to its dynamically allocated memory,
> such that the memory is leaked.
Hmm... but in this case, wouldn't calling this->~B() also invoke the
std::vector<int> member's destructor; which would free any memory the
vector had allocated? The memory used for the vector<int> itself would
stick around, of course (it's been destructed but not deallocated),
but any cleanup it did in it's destructor would happen. At least,
that's what I expect; because you can use placement new on object
types that have other objects as members, and the only thing you need
to do to clean up (aside from actually flagging the memory you used
for it as free in whatever way is appropriate) is call the object's
destructor.
But...
On Feb 27, 2:04 am, "Alf P. Steinbach" <al...@start.no> wrote:
> In particular it's problematic if anyone ever derives from B: then you're
> constructing the wrong kind of object.
... that makes sense. Oh well... guess that kills that idea (Triple-
DES wrote the same thing)! Thanks for pointing that out.
> Assignment operator or not has nothing to with the above issue.
In the above case, no, since the default assignment operator works.
But if you say "(*this) = B()", and B is something like, say:
class B {
// ...
int *some_array_that_i_allocated;
};
Then wouldn't you have to make sure you implemented a proper
assignment operator that let "(*this) = B()" free
"some_array_that_i_allocated" first?
Jason
Alf P. Steinbach
> On Feb 27, 2:04 am, "Alf P. Steinbach" <al...@start.no> wrote:
>> In particular it's problematic if anyone ever derives from B: then you're
>> constructing the wrong kind of object.
>
> ... that makes sense. Oh well... guess that kills that idea (Triple-
> DES wrote the same thing)! Thanks for pointing that out.
>
>> Assignment operator or not has nothing to with the above issue.
>
> In the above case, no, since the default assignment operator works.
> But if you say "(*this) = B()", and B is something like, say:
>
> class B {
> // ...
> int *some_array_that_i_allocated;
> };
>
> Then wouldn't you have to make sure you implemented a proper
> assignment operator that let "(*this) = B()" free
> "some_array_that_i_allocated" first?
Assuming it was more realistic (the above easily fixed using std::vector): yes.
Check out the swap idiom for implementing assignment operator in terms of copy
constructor.
Juha Nieminen
> reset() { *this = B(); }
Btw, a bit related to that, can anyone spot some problem with this:
A::A(const A& rhs)
{
*this = rhs;
}
A& operator=(const A& rhs)
{
// Tons of member variable assignments here
}
The idea would be, of course, to save writing all the member variable
initializations twice.
(I really wish there was a way to call the compiler-generated "default
copy constructor" and the "default assignment operator" from your own
versions, so that you only had to concentrate on those members for which
the default assignment is not enough. The problem with having to
construct/assing every single member manually is that if you add a new
member variable in the future you have to remember to add it to both the
copy constructor and assignment operator. If you forget, the compiler
won't give you even a warning, and the program may malfunction in
strange ways, which will sometimes be very hard to debug.)
Alf P. Steinbach
> Alf P. Steinbach wrote:
>> reset() { *this = B(); }
>
> Btw, a bit related to that, can anyone spot some problem with this:
>
> A::A(const A& rhs)
> {
> *this = rhs;
> }
>
>
> A& operator=(const A& rhs)
> {
> // Tons of member variable assignments here
> }
>
> The idea would be, of course, to save writing all the member variable
> initializations twice.
It is again a problem with derived classes, but here "only" with efficiency and
expectations of maintainance programmers (the latter the most serious).
Another problem is that it requires data members to be default-constructible.
> (I really wish there was a way to call the compiler-generated "default
> copy constructor" and the "default assignment operator" from your own
> versions, so that you only had to concentrate on those members for which
> the default assignment is not enough. The problem with having to
> construct/assing every single member manually is that if you add a new
> member variable in the future you have to remember to add it to both the
> copy constructor and assignment operator. If you forget, the compiler
> won't give you even a warning, and the program may malfunction in
> strange ways, which will sometimes be very hard to debug.)
That's what the swap idiom for assignment is all about (in addition to exception
safety).
Jeff Schwab
> * jason.c...@gmail.com:
>> On Feb 26, 11:12 pm, Jeff Schwab <j...@schwabcenter.com> wrote:
>>> Alf P. Steinbach wrote:
>>>> * Christopher:
>>>> reset() { *this = B(); }
>>> I've never seen that before, but it does seem like a neat idea. :)
>>>
>>> It seems like it would be (generally speaking) twice as slow as manual
>>> member-by-member initialization
> No. Thinking about that would be premature optimization.
Whether it's slower is a completely separate issue from whether its
speed matters, or whether we "should" be thinking about it yet.
James Kanze
> > Alf P. Steinbach wrote:
> > > * Christopher:
> > > reset() { *this = B(); }
> > I've never seen that before, but it does seem like a neat
> > idea. :)
> > It seems like it would be (generally speaking) twice as slow
> > as manual member-by-member initialization, because of (a)
> > the temporary object's constructor, plus (b) the assignment
> > operator.
If it makes a measurable difference, make the functions inline,
and let the compiler's optimizer take care of it.
> Just now I managed to get this working using placement new. Is
> there anything bad that can happen with the following code
> (assuming the constructor does not throw any exceptions):
> class B {
> public:
> B () { }
> ~B () { }
> void Reset () {
> this->~B();
> new (this) B();
> }
> };
Well assuming the constructor doesn't throw any exceptions, and
assuming that no one ever derives from the class, and probably
assuming one or two other things, it's OK.
In general, there are just too many assumptions in there for me
to ever want to see it in production code.
> I've done a good amount of testing on that just now and it
> seems to work fine (even when B has other complex object
> members, etc.).
Try adding a virtual member (say a virtual destructor), then
deriving. I think you'll find some mighty strange behavior.
And of course, it's very, very hard to guarantee that a
constructor doesn't throw.
--
James Kanze (GABI Software) email:james...@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
James Kanze
> * Juha Nieminen:
> > Alf P. Steinbach wrote:
> >> reset() { *this = B(); }
> > Btw, a bit related to that, can anyone spot some problem
> > with this:
> > A::A(const A& rhs)
> > {
> > *this = rhs;
> > }
> > A& operator=(const A& rhs)
> > {
> > // Tons of member variable assignments here
> > }
> > The idea would be, of course, to save writing all the member variable
> > initializations twice.
> It is again a problem with derived classes, but here "only"
> with efficiency and expectations of maintainance programmers
> (the latter the most serious).
I don't think it's really a problem with derived classes,
because at this point, they don't exist yet. (Their constructor
hasn't yet been called.) However...
> Another problem is that it requires data members to be
> default-constructible.
Worse: if some of the data has do nothing constructors, and the
operator= expects it to be initialized. Pointers to dynamic
memory would be a classical case---if the operator= results in
the old values being passed to delete, delete's not going to
like getting passed some random bits.
As you say...
> > (I really wish there was a way to call the
> > compiler-generated "default copy constructor" and the
> > "default assignment operator" from your own versions, so
> > that you only had to concentrate on those members for which
> > the default assignment is not enough. The problem with
> > having to construct/assing every single member manually is
> > that if you add a new member variable in the future you have
> > to remember to add it to both the copy constructor and
> > assignment operator. If you forget, the compiler won't give
> > you even a warning, and the program may malfunction in
> > strange ways, which will sometimes be very hard to debug.)
> That's what the swap idiom for assignment is all about (in
> addition to exception safety).
In simple cases, you may be able to do even simpler than the
swap idiom, but it is the guaranteed general solution (provided
all of the members support it as well).
jason.c...@gmail.com
13sa67q...@corp.supernews.com...
> Check out the swap idiom for implementing assignment operator in terms of copy
> constructor.
Is that something like this (from http://www.gotw.ca/gotw/059.htm):
T& T::operator=( const T& other )
{
T temp( other ); // do all the work off to the side
Swap( temp ); // then "commit" the work using
return *this; // nonthrowing operations only
}
And one advantage is there's really not much to do in the = operator,
since you are using the constructors to do a lot the work? So a
Reset() might simply be:
void T::Reset () {
Swap(T());
}
But in that case... I mean, is there a way to implement Swap() that
doesn't involve explicitly typing out code to swap all of the members?
Since you just need to do a "shallow" swap... is something like this
reliable:
void T::Swap (T &other) {
char temp[sizeof(T)];
memcpy(temp, &other, sizeof(T));
memcpy(&other, *this, sizeof(T));
memcpy(*this, temp, sizeof(T));
}
That seems like a horrible way to do it... also, what if "other" is
something derived from T? Is there a way to just use the compilers
default assignment operator even when you've defined your own? Or does
it always make sense to use the "pimpl" idiom when implementing a swap
function, so that you don't have to do something like that?
Jason
Alf P. Steinbach
> "Alf P. Steinbach" <al...@start.no> wrote in message news:
> 13sa67q...@corp.supernews.com...
>> Check out the swap idiom for implementing assignment operator in terms of copy
>> constructor.
>
> Is that something like this (from http://www.gotw.ca/gotw/059.htm):
>
> T& T::operator=( const T& other )
> {
> T temp( other ); // do all the work off to the side
> Swap( temp ); // then "commit" the work using
> return *this; // nonthrowing operations only
> }
Yes, except you can do it neater:
T& operator=( T other )
{
swap( other );
return *this;
}
> And one advantage is there's really not much to do in the = operator,
> since you are using the constructors to do a lot the work?
Main advantages are exception safety and avoiding redundant code (always a
source of bugs and extra work).
> So a
> Reset() might simply be:
>
> void T::Reset () {
> Swap(T());
> }
Well no, it would have to be
void reset() { T().swap( *this ); }
> But in that case... I mean, is there a way to implement Swap() that
> doesn't involve explicitly typing out code to swap all of the members?
> Since you just need to do a "shallow" swap... is something like this
> reliable:
>
> void T::Swap (T &other) {
> char temp[sizeof(T)];
> memcpy(temp, &other, sizeof(T));
> memcpy(&other, *this, sizeof(T));
> memcpy(*this, temp, sizeof(T));
> }
It all depends. Some member might have a pointer back to the object, say.
However, there is a nice little elegant facility for iterating through members
in a little side project of Boost.Spirit, I forget the name (Poenix?), and
possibly that might serve as foundation for a general implementation?
> That seems like a horrible way to do it...
memcpy, yes, horrible.
> also, what if "other" is something derived from T?
The swap idiom takes care of that, whereas other idioms may not necessarily take
care of that.
> Is there a way to just use the compilers
> default assignment operator even when you've defined your own?
Depends which assignment operator you have defined. :-) But if you have defined
the copy assignment operator, then no. On the other hand, the compiler will use
your copy assignment operator when it defines one for a derived class.
> Or does
> it always make sense to use the "pimpl" idiom when implementing a swap
> function, so that you don't have to do something like that?
I don't see what the PIMPL idiom has to do with this, but possibly you mean
having a pointer to the real object state so that you only need to swap
pointers. Well that's what std::vector etc. has. And they also have swap
member functions, so, very efficient.
Alf P. Steinbach
> On Feb 27, 11:48 am, "Alf P. Steinbach" <al...@start.no> wrote:
>> * Juha Nieminen:
>>> Alf P. Steinbach wrote:
>>>> reset() { *this = B(); }
>
>>> Btw, a bit related to that, can anyone spot some problem
>>> with this:
>
>>> A::A(const A& rhs)
>>> {
>>> *this = rhs;
>>> }
>
>>> A& operator=(const A& rhs)
>>> {
>>> // Tons of member variable assignments here
>>> }
>
>>> The idea would be, of course, to save writing all the member variable
>>> initializations twice.
>
>> It is again a problem with derived classes, but here "only"
>> with efficiency and expectations of maintainance programmers
>> (the latter the most serious).
>
> I don't think it's really a problem with derived classes,
> because at this point, they don't exist yet. (Their constructor
> hasn't yet been called.) However...
Well, I was thinking of the above as a pattern, used also in derived classes.
Then a call chain :-)
Derived::Derived
-> Base::Base
-> Base::Base[init-list] // Base members def-inits
-> Base::Base[body]
-> Base::operator=() // Base members assigned
-> Derived::Derived[init-list] // Derived members def-inits.
-> Derived::Derived[body]
-> Derived::operator=()
-> Base::operator=() // Base members assigned 2nd time.
-> [assignment statements] // Derived members assigned
[snip]
> In simple cases, you may be able to do even simpler than the
> swap idiom
?
>, but it is the guaranteed general solution (provided
> all of the members support it as well).
Cheers,
jason.c...@gmail.com
>> So a
>> Reset() might simply be:
>>
>> void T::Reset () {
>> Swap(T());
>> }
>
> Well no, it would have to be
>
> void reset() { T().swap( *this ); }
How is that different from "this->Swap(T())"?
> However, there is a nice little elegant facility for iterating through members
> in a little side project of Boost.Spirit, I forget the name (Poenix?), and
> possibly that might serve as foundation for a general implementation?
Phoenix is here:
http://www.boost.org/libs/spirit/phoenix/index.html
From what I can tell skimming through the introduction now, it looks
like a project to somehow facilitate functional programming in C++. I
bet there's something good in there somewhere. I'll post back here if
I find a way to do it -- "Phoenix" looks like the kind of thing that
could distract me for hours so I'm going to have to put it off until
the weekend :) .
>> also, what if "other" is something derived from T?
>
> The swap idiom takes care of that, whereas other idioms may not necessarily take
> care of that.
So... if I have:
void T::Swap (T &other) {
}
And 'other' is something derived from T... my Swap() would only swap
the members of 'other' that were in the T base, leaving any new
members in the derived class untouched. Is that behavior part of the
"swap idiom" (only swapping the members that two objects have in
common, leaving the others untouched) (it does seem reasonable)?
>> Or does
>> it always make sense to use the "pimpl" idiom when implementing a swap
>> function, so that you don't have to do something like that?
>
> I don't see what the PIMPL idiom has to do with this, but possibly you mean
> having a pointer to the real object state so that you only need to swap
> pointers.
That is what I meant. Sorry for being unclear -- I was secretly still
looking at http://www.gotw.ca/gotw/059.htm and going off the rest of
the example there; where they did just that -- swapped pointers to the
object's state (and it's straight forward to update any state pointer
back to the object too; just as long as, like you mentioned above,
none of the members in the object state point back to the object
itself, which shouldn't happen unless you were sloppy).
Thanks for your time,
Jason
Alf P. Steinbach
> Thanks for the detailed response!
>
>>> So a
>>> Reset() might simply be:
>>>
>>> void T::Reset () {
>>> Swap(T());
>>> }
>> Well no, it would have to be
>>
>> void reset() { T().swap( *this ); }
>
> How is that different from "this->Swap(T())"?
If you just try it you'll see... ;-)
>> However, there is a nice little elegant facility for iterating through members
>> in a little side project of Boost.Spirit, I forget the name (Poenix?), and
>> possibly that might serve as foundation for a general implementation?
>
> Phoenix is here:
>
> http://www.boost.org/libs/spirit/phoenix/index.html
>
> From what I can tell skimming through the introduction now, it looks
> like a project to somehow facilitate functional programming in C++. I
> bet there's something good in there somewhere. I'll post back here if
> I find a way to do it -- "Phoenix" looks like the kind of thing that
> could distract me for hours so I'm going to have to put it off until
> the weekend :) .
I checked.
I think what I was thinking of was Fusion 2.0,
<url:
http://spirit.sourceforge.net/dl_more/fusion_v2/libs/fusion/doc/html/fusion/extension/ext_full.html>.
Not sure how that can be simplified down, though.
>>> also, what if "other" is something derived from T?
>> The swap idiom takes care of that, whereas other idioms may not necessarily take
>> care of that.
>
> So... if I have:
>
> void T::Swap (T &other) {
> }
>
> And 'other' is something derived from T... my Swap() would only swap
> the members of 'other' that were in the T base, leaving any new
> members in the derived class untouched. Is that behavior part of the
> "swap idiom" (only swapping the members that two objects have in
> common, leaving the others untouched) (it does seem reasonable)?
Yeah, it's just usual C++ slicing.
However, for assignment nothing happens to the source object.
It's a (sliced or converted to) copy that's swapped.
James Kanze
> > Check out the swap idiom for implementing assignment
> > operator in terms of copy constructor.
> Is that something like this (fromhttp://www.gotw.ca/gotw/059.htm):
> T& T::operator=( const T& other )
> {
> T temp( other ); // do all the work off to the side
> Swap( temp ); // then "commit" the work using
> return *this; // nonthrowing operations only
> }
That's the way I write it. Two other frequent alternatives are:
T&
T::operator=(
T const& other )
{
T( other ).swap( *this ) ;
return *this ;
}
and
T&
T::operator=(
T other )
{
swap( other ) ;
return *this ;
}
(Note that in the last case, the copy is actually done by the
caller, so you don't need it in the operator= function.)
> And one advantage is there's really not much to do in the =
> operator, since you are using the constructors to do a lot the
> work?
The real advantage is that it is the simplest way to ensure that
all operations which can fail take place before you modify
anything in the object being assigned to.
> So a Reset() might simply be:
> void T::Reset () {
> Swap(T());
> }
> But in that case... I mean, is there a way to implement Swap()
> that doesn't involve explicitly typing out code to swap all of
> the members?
Not really, since some of the members may have specialized swap
algorithms.
> Since you just need to do a "shallow" swap... is something
> like this reliable:
> void T::Swap (T &other) {
> char temp[sizeof(T)];
> memcpy(temp, &other, sizeof(T));
> memcpy(&other, *this, sizeof(T));
> memcpy(*this, temp, sizeof(T));
> }
No.
> That seems like a horrible way to do it...
It is, since it involves undefined behavior.
Note that it's entirely possible that the swap idiom cannot be
used for certain classes---if there's no way to do a guaranteed
nothrow swap of some base class or member.
> also, what if "other" is something derived from T?
What about it? Assignment cannot change the type of the object
being assigned, so you only assign the T part of the derived
object. (In general, assignment and polymorphism don't work
well together, and if a class is designed to be used as a base
class, it's usually best to inhibit assignment in it.)
> Is there a way to just use the compilers default assignment
> operator even when you've defined your own?
No. Presumably, if you provide your own assignment operator,
it's because the compiler's default assignment operator doesn't
do what you want.
> Or does it always make sense to use the "pimpl" idiom when
> implementing a swap function, so that you don't have to do
> something like that?
In a certain sense, it always makes sense to use the swap idiom
for assignment if you use the compilation firewall idiom.
Except that in the case of the compilation firewall, you're
dealing with an easily recognizable idiom, and even the standard
implementation of operator= is so simple that I don't find it
worth the bother. Generally speaking:
-- if the class corresponds to an established idiom (like the
compilation firewall), then use the assignment operator for
the idiom, otherwise,
-- if all members support value nothrow value assignment, then
the compiler defined assignment operator is all you need; no
point in the swap idiom, otherwise,
-- if all of the members support value assignment, and you
don't need the strong exception guarantee, then the compiler
defined assignment operator is all you need as well,
otherwise,
-- if all of the members are either non-class types, or class
types which support a nothrow swap (generally, which have a
member function swap), then use the swap idiom, using
std::swap for the non-class types, and the nothrow swap for
the class types, otherwise
-- you're on your own, and it may be difficult.
In practice, if all of the libraries you're using understand the
swap idiom, and have been written in consequence, the fourth
case will be rather frequent. If you have to deal with legacy
libraries, however, you may find that you fall into the fifth
case more often than you'd like. In such cases, I find that it
often is convenient to use the compilation firewall idiom in
such cases, to move them up to the first case above.
James Kanze
> >> So a Reset() might simply be:
> >> void T::Reset () {
> >> Swap(T());
> >> }
> > Well no, it would have to be
> > void reset() { T().swap( *this ); }
> How is that different from "this->Swap(T())"?
It's legal code. It's hard to imagine swap taking anything but
a non-const reference as an argument, and of course, you can't
initialize a non-const reference with a temporary (even if some
compilers fail to detect the error).
[...]
> >> also, what if "other" is something derived from T?
> > The swap idiom takes care of that, whereas other idioms may
> > not necessarily take care of that.
(Not knowing what the problem is, I can't say whether swap takes
care of it or not. I don't see how swap has any impact one way
or the other, but perhaps Alf sees a problem that I don't.)
> So... if I have:
> void T::Swap (T &other) {
> }
> And 'other' is something derived from T... my Swap() would
> only swap the members of 'other' that were in the T base,
> leaving any new members in the derived class untouched.
How could it do otherwise, given that it doesn't know anything
about the derived class?
But of course, that question is moot in the swap idiom for
assignment, since other will never be an object with a derived
class type.
> Is that behavior part of the "swap idiom" (only swapping the
> members that two objects have in common, leaving the others
> untouched) (it does seem reasonable)?
It doesn't seem reasonable to me to support assignment if the
class is to be used as a base class.
> >> Or does it always make sense to use the "pimpl" idiom when
> >> implementing a swap function, so that you don't have to do
> >> something like that?
> > I don't see what the PIMPL idiom has to do with this, but
> > possibly you mean having a pointer to the real object state
> > so that you only need to swap pointers.
> That is what I meant. Sorry for being unclear -- I was
> secretly still looking athttp://www.gotw.ca/gotw/059.htmand
> going off the rest of the example there; where they did just
> that -- swapped pointers to the object's state (and it's
> straight forward to update any state pointer back to the
> object too; just as long as, like you mentioned above, none of
> the members in the object state point back to the object
> itself, which shouldn't happen unless you were sloppy).
In the case of the compilation firewall idiom, you can get even
simpler than swap. The standard operator= in that case is:
T&
T::operator=( T const* other )
{
Impl* tmp = new Impl( *other.myImpl ) ;
// or = other.myImpl->clone()...
delete myImpl ;
my Impl = tmp ;
return *this ;
}
The swap idiom is also quite acceptable in this case, but
perhaps for historical reasons, I find the above more idiomatic.
James Kanze
> * jason.cipri...@gmail.com:
> > Thanks for the detailed response!
> >>> So a
> >>> Reset() might simply be:
> >>> void T::Reset () {
> >>> Swap(T());
> >>> }
> >> Well no, it would have to be
> >> void reset() { T().swap( *this ); }
> > How is that different from "this->Swap(T())"?
> If you just try it you'll see... ;-)
Maybe, maybe not. I think that there are still some widespread
compilers which let it through, without so much as a warning
(although it has been illegal now for almost 20 years).
James Kanze
Good point. Arguable, if your class supports assignment, then
assigning the same value twice shouldn't cause any problems.
But it's certainly something I'd prefer avoiding, just in case.
(Of course, arguably, if your class is being used as a base
class, it shouldn't support assignment at all. But that's
another issue.)
> [snip]
> > In simple cases, you may be able to do even simpler than the
> > swap idiom
> ?
You wouldn't really use the swap idiom for something like
complex, would you? (For that matter, would you ever use it if
the compiler generated assignment operator would work?)
Even in the case of the compilation firewall idiom, I tend to
just use the classical assignment operator---probably because I
was using the compilation firewall idiom long before I'd ever
heard of the swap idiom. (It was called the Cheshire cat idiom
back then---the implementation just disappears, until all you
see is its smile, and then not even that.) It's in my fingers.
But if I were to change, I'd use a boost::scoped_ptr, and
simply:
T&
T::operator=( T const& other )
{
myImpl.reset( new Impl( *other.myImpl ) ) ;
return *this ;
}
You could use the swap idiom, but it seems more complex here,
and it is certainly not necessary.
Note that in the case of the compilation firewall idiom, part of
the reason why the swap idiom isn't necessary is because the
compilation firewall idiom is an established pattern. I don't
have to think about things like exception safety issues, because
the "standard" solutions are known to be exception safe. In a
more general case, I might adopt the swap idiom simply because
it means that I know I have exception safety, without having to
think any further about it.
Not that I don't like thinking, but there are always enough
other things to think about, for which there are no simple
solutions, to keep me occupied. Anytime there's a simple
solution which is guaranteed to work, I'm all for it.