Newbie: Exception safe assignment

B

Barry

Hi all,

I'm learning C++ and am currently trying to get my head around memory
leakage.

I have a class, C, which contains a number of vectors, as follows -

vector<A> a;
vector<B> b;

I'm trying to implement an assignment operator for this class which is
exception safe and am wondering if the following meets this criteria
and if I'm doing it correctly -

C& C::eek:perator=(const C& c)
{
C temp(c);
temp.swap(*this);
return *this;
}

void C::swap (C &c) throw ()
{
std::swap(this->a,c.a);
std::swap(this->b,c.b);
}

Thanks very much for your help,

Barry.
 
F

Francesco

Hi all,

I'm learning C++ and am currently trying to get my head around memory
leakage.

I have a class, C, which contains a number of vectors, as follows -

vector<A> a;
vector<B> b;

I'm trying to implement an assignment operator for this class which is
exception safe and am wondering if the following meets this criteria
and if I'm doing it correctly -

C& C::eek:perator=(const C& c)
{
C temp(c);
temp.swap(*this);
return *this;

}

void C::swap (C &c) throw ()
{
std::swap(this->a,c.a);
std::swap(this->b,c.b);

}

Thanks very much for your help,

Barry.

Hi Barry,
as long as you don't use "new" - that is, as long as you don't create
dynamic objects, you aren't going to have any leakage.

About exception safety, in this case you'd have to check whether
std::swap throws. If it does, you should intercept those exceptions,
so that the "throw()" specification is fulfilled.

By the way, what happens if I execute the following code on your
class?

-------
C c;
c = c;
-------

You might want to add a check to intercept the case I mentioned.
Also, you might prefer to use the vector's swap method instead of
std::swap.

Best regards,
Francesco
 
B

Barry

Hi Barry,
as long as you don't use "new" - that is, as long as you don't create
dynamic objects, you aren't going to have any leakage.

About exception safety, in this case you'd have to check whether
std::swap throws. If it does, you should intercept those exceptions,
so that the "throw()" specification is fulfilled.

By the way, what happens if I execute the following code on your
class?

-------
C c;
c = c;
-------

You might want to add a check to intercept the case I mentioned.
Also, you might prefer to use the vector's swap method instead of
std::swap.

Best regards,
Francesco

Thanks Francesco!

How do I know if std::swap throws?
 
J

Jonathan Lee

C& C::eek:perator=(const C& c)
{
  C temp(c);
  temp.swap(*this);
  return *this;

}

Hello Barry,
Out of curiousity, why are you doing a copy construct and then a
swap to accomplish assignment? Why not just assign field by field? I
guess maybe there's a small code re-use advantage. But there's a small
allocate and delete disadvantage in using the automatic variable,
"temp".

As for exceptions, the copy constructor has the potential to throw;
there is at least the possibility that you run out of memory. Unless
something strange happens inside the constructor, that should be about
your only concern. A straight assignment field by field would help
reduce that possibility, but if memory won't hold "c", "temp" and
*this then it probably won't hold "c" and *this, anyway.

C::swap shouldn't throw. I'm pretty sure std::swap for vector<T> just
swaps some pointers and the current size, max size, etc. fields.

--Jonathan
 
J

Jerry Coffin

(e-mail address removed)>, (e-mail address removed)
says...

[ ... ]
By the way, what happens if I execute the following code on your
class?

An exception-safe assignment operator also generally works in the
case of self-assignment. You may still want to prevent self
assignment for performance reasons though.
You might want to add a check to intercept the case I mentioned.
Also, you might prefer to use the vector's swap method instead of
std::swap.

There's really no reason to do so -- the standard requires a
specialization of std::swap for std::vector, so something like:
'std::swap(x, y);' is equivalent to 'x.swap(y);' when/if x and y are
both vectors.
 
J

Jerry Coffin

(e-mail address removed)>, (e-mail address removed)
says...

[ ... ]
How do I know if std::swap throws?

In this case, the route to knowing that is a bit convoluted. The
standard requires a specialization, so std::swap for two vectors is
equivalent to using one the swap member function of the left operand.

Therefore, you have to look at the documentation for the swap member
function, in this case in the section under 'container requirements',
which, among other things, says (§23.1/10):

? no swap() function throws an exception unless that exception is
thrown by the copy constructor or assignment operator of the
container?s Compare object (if any; see 23.1.2).

Since a vector doesn't have a Compare object, swap never throws for
it.
 
J

Joshua Maurice

Hello Barry,
  Out of curiousity, why are you doing a copy construct and then a
swap to accomplish assignment? Why not just assign field by field? I
guess maybe there's a small code re-use advantage. But there's a small
allocate and delete disadvantage in using the automatic variable,
"temp".

Because it's the common idiom, aka the copy-swap idiom. You get code
reuse, and actually with an optimizing compiler little to no overhead.
On assignment, you need to destroy this and then copy the argument to
this. The copy swap does exactly that. It copies the argument to a
temp, then swaps the internals of temp and this, meaning the old this
aka new temp will be destroyed, and the new this is a copy of the
argument. It generally works out quite well.
 
J

Joshua Maurice

I'm learning C++ and am currently trying to get my head around memory
leakage.

I have a class, C, which contains a number of vectors, as follows -

vector<A> a;
vector<B> b;

I'm trying to implement an assignment operator for this class which is
exception safe and am wondering if the following meets this criteria
and if I'm doing it correctly -

C& C::eek:perator=(const C& c)
{
  C temp(c);
  temp.swap(*this);
  return *this;
}

void C::swap (C &c) throw ()
{
  std::swap(this->a,c.a);
  std::swap(this->b,c.b);
}

In this case, there is no magic under the hood. operator= can throw
iff the copy constructor C::C throws or C::swap throws. C::swap throws
iff std::swap throws for a or b.

Also, I'm not sure I would suggest using the no-throws specification
"throw()" if you're unsure if it can throw. First off, some compilers
(I'm looking at you windows) don't actually correctly implement it.
The correct implementation is to call terminate or something if an
exception would be thrown and escape swap. (Windows visual studios
IIRC lets it escape without calling terminate or whatever.) I would
suggest using throw() only on functions which are actually no-throws,
and not as a runtime check as guaranteed by the standard. It's both
documentation and an optimization allowance to some implementations.

Small aside: Last time I checked, there is a "defect report" related
to swap use. If a or b are types defined in another library, then you
will be using std::swap and not the swap from that library. std::swap
will probably make a copy instead of efficiently swapping the
internals like the swap from the library for "a" would do. I think the
current suggested course of action is use std::swap for built-in types
or types from the standard library, but otherwise do:
void C::swap (C &c) throw ()
{
using std::swap;
swap(this->a,c.a);
swap(this->b,c.b);
}
With Koenig lookup (aka ADL), it will use swap from the library of "a"
if such a swap exists. Otherwise it will use std::swap. This is
exactly what you want.
 
G

Gert-Jan de Vos

(e-mail address removed)>, (e-mail address removed)
says...

[ ... ]
By the way, what happens if I execute the following code on your
class?

An exception-safe assignment operator also generally works in the
case of self-assignment. You may still want to prevent self
assignment for performance reasons though.

Which would be a pessimization. The copy/swap idiom doesn't need an if
to special case self assignment. This makes typical cases faster at
the expense of the a typical self assignment case that needs a
redundant copy. I find it hard to imagine a case where optimizing for
self assignment would optimize the program as a whole.

Other than that, once you understand this idiom I find it more simple
to read and write and verify for correctness than the alternative.
 
F

Francesco

(e-mail address removed)>, (e-mail address removed)
says...
[ ... ]
By the way, what happens if I execute the following code on your
class?
An exception-safe assignment operator also generally works in the
case of self-assignment. You may still want to prevent self
assignment for performance reasons though.

Which would be a pessimization. The copy/swap idiom doesn't need an if
to special case self assignment. This makes typical cases faster at
the expense of the a typical self assignment case that needs a
redundant copy. I find it hard to imagine a case where optimizing for
self assignment would optimize the program as a whole.

Unless the program as a whole does a lot of self-assignments ;-)
Just kidding, of course, you're right, it's better not to intercept
that case when using the copy-swap idiom. It was my fault, in first
place, to suggest adding that check, thanks for fixing my bad advice.

Best regards,
Francesco
 
F

Francesco

(e-mail address removed)>, (e-mail address removed)
says...

There's really no reason to do so -- the standard requires a
specialization of std::swap for std::vector, so something like:
'std::swap(x, y);' is equivalent to 'x.swap(y);' when/if x and y are
both vectors.

I didn't know about that specialization, thanks a lot for pointing
that out and fixing my bad advice.

Best regards,
Francesco
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,582
Members
45,065
Latest member
OrderGreenAcreCBD

Latest Threads

Top