Sign in to follow this  
Lode

copying classes

Recommended Posts

Lode    1003
Say I have class A and class B. Class A has NO custom copy contructor or assignement operator defined. B HAS a custom copy constructor and assignement operator defined. Class A contains an object of class B in it. Now I do:
A x, y = {some parameters};


x = y; //remember that A had no custom assignement operator
Will the object of type B inside A, be copied bit by bit (so pointers are not updated if necessary), or will the custom assignement operator of B be called (B has one after all as I mentioned above)? In other words, if class A itself does not need a deep copy, but class B does need a deep copy and has it defined, and class A contains a class B in it, will the default shallow copy of A call B's deep copy, or not? Also, because I don't want to type the same thing twice, if the code of the copy constructor and assignement operator are exactly the same, is there a way to have to type it only once?

Share this post


Link to post
Share on other sites
Fruny    1658
B's copy constructor will be called. The default copy operation isn't a bitwise copy but a memberwise copy.

In other words, if all your members either are basic type or have their own copy constructor (destructor, assignment operator), the enclosing class doesn't need you to define a copy constructor (destructor, assignment operator).

Quote:
Also, because I don't want to type the same thing twice, if the code of the copy constructor and assignement operator are exactly the same, is there a way to have to type it only once?


Yes and no... The safest way is to implement the assignment operator as a copy construction of a temporary and a swap with the target variable. You will, of course, have to implement the swap function... probably in terms of other member swaps.

Share this post


Link to post
Share on other sites
Lode    1003
Quote:
Original post by Fruny
Yes and no... The safest way is to implement the assignment operator as a copy construction of a temporary and a swap with the target variable. You will, of course, have to implement the swap function... probably in terms of other member swaps.


Do you think it could be possible to call the assignement operator from the copy constructor (if I type somthing like "this = the_other_one" in the copy constructor and define the operator= function)?

Share this post


Link to post
Share on other sites
desertcube    514
I believe Fruny ment something like this:

#include <algorithm>

class MyClass
{
public:
MyClass(const MyClass & rhs) {/*Copy constructor's code*/}

MyClass & operator=(const MyClass & rhs)
{
MyClass temp(rhs); //Create a copy
std::swap(classMemberVariables, temp.classMemberVariables); //Swap the variables
return *this;
}
};


The reason to create a temp is to prevent problems with self-assigments if resources have to be freed, e.g.

MyClass instance;
//Later on in the function
MyClass & anotherInstance = instance;
//Further down
instance = anotherInstance;

Pretend that your class contains a pointer, what happens if you did this?

MyClass & operator=(const MyClass & rhs)
{
this->myPointer = rhs.myPointer;
rhs.myPointer = 0; //Oh dear, rhs == *this!! We've creater a memory leak
return *this;
}

Hope that helps

Share this post


Link to post
Share on other sites
chollida1    532
desertcube, I've always just compared the other object to myself, is there a reason why you would prefer the swap approach to this?? It seems like a neat approach but if you assigning to your self should be disallowed I think the

if ( rhs !== this ) approach is more clear.

Cheers
Chris

Share this post


Link to post
Share on other sites
desertcube    514
Quote:
Original post by chollida1
desertcube, I've always just compared the other object to myself, is there a reason why you would prefer the swap approach to this?? It seems like a neat approach but if you assigning to your self should be disallowed I think the

if ( rhs !== this ) approach is more clear.

Cheers
Chris

I'm not entirely sure in all honestly!! I read it somewhere and thought it was smart!

A quick google gave me this, which basically says that using a swap function makes sure that the assignment will be safe from exceptions (well, it will be uneffected if an exception occurs.) Hopefully, this should make it a but clearer:

class MyClass
{
public:
MyClass() {itsPointer = new int[8];}
~MyClass() {delete [] itsPointer;}

MyClass(const MyClass & rhs);
MyClass & operator=(const MyClass & rhs);

//Functions
private:
int * itsPointer;
};

MyClass::MyClass(const MyClass & rhs)
{
itsPointer = new int[8]; //This may throw if there isn't enough memory!
std::copy(&rhs.itsPointer[0], &rhs.itsPointer[7], itsPointer); //Copy over the values
}
MyClass & MyClass::operator=(const MyClass & rhs)
{
//This statement may throw if the copy constructor fails,
//but if it does, our class will be unaffected, as we haven't changed anything!
MyClass temp(rhs);
//If you have a lot of member variables, then you'd probably create a seperate swap function.
std::swap(itsPointer, temp.itsPointer);
return *this; //Allow chaining
//As temp goes out of scope, its destructor will get called,
//but since it's pointer is now our old pointer, that peice of memory will get freed instead!
}



Share this post


Link to post
Share on other sites
petewood    819
Quote:
Original post by chollida1
desertcube, I've always just compared the other object to myself, is there a reason why you would prefer the swap approach to this?? It seems like a neat approach but if you assigning to your self should be disallowed I think the

if ( rhs !== this ) approach is more clear.


If your code relies upon checking for self assignment then it cannot be made exception safe. See GotW #23: Any copy assignment that "must" check for self-assignment is not exception-safe.

Checking for self assignment is a useful optimisation, however. But it shouldn't be necessary for the correct running of the code.

The original question comes up a lot as people are trying to avoid writing the same code twice (or copying and pasting). Swap is a useful member function to have on any class and makes it easy to implement the assignment operator in terms of the copy constructor and the swap function.


class MyClass {
int memberA;
double memberB;
std::vector<double> memberC;

public:
MyClass() : memberA(4), memberB(17.5), memberC(20, 1.55) {}
void swap(MyClass& other) {
std::swap(memberA, other.memberA);
std::swap(memberB, other.memberB);
memberC.swap(other);//vector has its own swap member; more efficient than calling std::swap
}
MyClass(const MyClass & rhs) {
memberA = rhs.memberA;
memberB = rhs.memberB;
memberC = rhs.memberC;
}
MyClass& operator=(const MyClass& rhs) {
MyClass temp(rhs);
temp.swap(*this);
return *this;
}
};

Share this post


Link to post
Share on other sites
ChaosEngine    5185
petewood, shouldn't your copy constructor be implemented using the initialiser list rather then an assignment list?


MyClass(const MyClass & rhs) {
memberA = rhs.memberA;
memberB = rhs.memberB;
memberC = rhs.memberC;
}
// what if any of these variables are const? assignment will fail

// safer (and more efficient too)
MyClass(const MyClass & rhs)
: memberA(rhs.memberA)
, memberB(rhs.memberB)
, memberC(rhs.memberC)
{
}



fundamentally, I'd say that assignment and copy construction are two different things, even if they happen to contain similar code. I also fail to see the benefit in moving the work from the = operator to a swap function. There shouldn't be that much code repition anyway (i.e. if your class has 50 members, it's time to look at your class design!)

I'd disagree with that gotw article that copy construction should be implemented using an overloaded assignment op (or private copy function), as again, they are not the same thing. While it's initally fine if your class has POD's, if you add a UDT member that does something funny in it's copy/assignment you're in tonnes of trouble.

Share this post


Link to post
Share on other sites
Zahlman    1682
Quote:
Original post by Fruny
B's copy constructor will be called. The default copy operation isn't a bitwise copy but a memberwise copy.

In other words, if all your members either are basic type or have their own copy constructor (destructor, assignment operator), the enclosing class doesn't need you to define a copy constructor (destructor, assignment operator).


This is one more reason to use std::string, BTW: it removes one source of pointer members from your object, thus increasing the chance that you will be able to escape from the Tyranny of the Rule of Three. :)

Quote:

Quote:
Also, because I don't want to type the same thing twice, if the code of the copy constructor and assignement operator are exactly the same, is there a way to have to type it only once?


Yes and no... The safest way is to implement the assignment operator as a copy construction of a temporary and a swap with the target variable. You will, of course, have to implement the swap function... probably in terms of other member swaps.


Is std::swap not good enough in general? Won't that do a memberwise swap? o_O

Share this post


Link to post
Share on other sites
petewood    819
Quote:
Original post by ChaosEngine
petewood, shouldn't your copy constructor be implemented using the initialiser list rather then an assignment list?

Shh, you're making me look bad.

(c:

Share this post


Link to post
Share on other sites
mfawcett    373
Quote:
Original post by ChaosEngine
...snip...

I'd disagree with that gotw article that copy construction should be implemented using an overloaded assignment op (or private copy function), as again, they are not the same thing. While it's initally fine if your class has POD's, if you add a UDT member that does something funny in it's copy/assignment you're in tonnes of trouble.

I'm having trouble understanding why that would get you into trouble. It would still use the UDT's assignment operator, so what's the problem?

On another note, if your class implements a faster swap, like std::vector, it would be nice if std::swap used it. Currently it's like so:

// This copies the vectors 3 times!
// vec1 into a temp vec (which requires an allocation)
// vec2 into vec1 (may require an allocation)
// temp into vec2 (may require an allocation)
std::swap(vec1, vec2);

// Swaps 2 pointers.
vec1.swap(vec2);

Share this post


Link to post
Share on other sites
ChaosEngine    5185
Quote:
Original post by mfawcett
Quote:
Original post by ChaosEngine
...snip...

I'd disagree with that gotw article that copy construction should be implemented using an overloaded assignment op (or private copy function), as again, they are not the same thing. While it's initally fine if your class has POD's, if you add a UDT member that does something funny in it's copy/assignment you're in tonnes of trouble.

I'm having trouble understanding why that would get you into trouble. It would still use the UDT's assignment operator, so what's the problem?

On another note, if your class implements a faster swap, like std::vector, it would be nice if std::swap used it. Currently it's like so:

// This copies the vectors 3 times!
// vec1 into a temp vec (which requires an allocation)
// vec2 into vec1 (may require an allocation)
// temp into vec2 (may require an allocation)
std::swap(vec1, vec2);

// Swaps 2 pointers.
vec1.swap(vec2);


ok maybe not tonnes of trouble, but as you pointed out using swap when you only want to create a class is gonna add potentially significant overhead.

for example

class A
vector<int> m_list;
public:
A()
{
m_list.reserve(A_VERY_BIG_NUMBER);
}

A(const A& rhs)
: m_list(rhs.m_list)
{
}

A& operator=(const A& rhs)
{
m_list.assign(rhs.m_list.begin(), rhs.m_list.end());
return *this;
}

};



class B
A m_a;
public:
B(const B& rhs)
// we've called A() here for no reason
{
this->operator=(rhs);
}

B& operator=(const B& rhs)
{
m_a = rhs.m_a;
return *this;
}



ok that's a contrived example, but in big projects classes can often get large (not in number of members neccessarily, but in size from aggregates), and an unnecessary default ctor call is pointless.

Share this post


Link to post
Share on other sites
Fruny    1658
Quote:
Original post by ChaosEngine
ok that's a contrived example, but in big projects classes can often get large (not in number of members neccessarily, but in size from aggregates), and an unnecessary default ctor call is pointless.


vector's swap is a constant-time operation. That's the whole point. The elements need to get copied once anyway. In comparison the constructor's cost is negligible; the safety benefits aren't.

Share this post


Link to post
Share on other sites
ChaosEngine    5185
Quote:
Original post by Fruny
Quote:
Original post by ChaosEngine
ok that's a contrived example, but in big projects classes can often get large (not in number of members neccessarily, but in size from aggregates), and an unnecessary default ctor call is pointless.


vector's swap is a constant-time operation. That's the whole point. The elements need to get copied once anyway. In comparison the constructor's cost is negligible; the safety benefits aren't.


What I was getting at is that you don't necessarily know that the constructors cost in negligable, especially if working with a library with no source. The constructor might do something (perfectly valid for it's implementation) like acquiring a system resource.

But I take your point that, in the general case, the safety benefits outweigh the cost. I guess is my something new for today!

Share this post


Link to post
Share on other sites
Deyja    920
You can always specialize std::swap for your type if your type can implement swap faster.

Quote:
Is std::swap not good enough in general? Won't that do a memberwise swap? o_O


std::swap doesn't know a thing about the internals of the class. It just uses the copy constructor and = operator. If operator= happens to call std::swap on it's own type, you've got an infinite loop.

The swap-with-temporary is an accepted and proven idiom. It can't be beat (yet).

[edit]
Also; the swap idiom offers the strong exception guarantee. The actual work of copying the object is in the temporaries copy constructor. If the copy fails, and this constructor fails, no modification is made to the assignee object. The swap itself should be written such that it can never throw anything. Usually, it's just a matter of switching pointers and such.
[/edit]

Share this post


Link to post
Share on other sites
Zahlman    1682
Quote:
Original post by Deyja
You can always specialize std::swap for your type if your type can implement swap faster.

Quote:
Is std::swap not good enough in general? Won't that do a memberwise swap? o_O


std::swap doesn't know a thing about the internals of the class. It just uses the copy constructor and = operator. If operator= happens to call std::swap on it's own type, you've got an infinite loop.


Uh. Damn. I look really bad in a couple of recent FB posts, then. Egg on my face, oh yes.

That sucks. Is there any good way to go about generalizing things then (i.e. create a templated assignment op using copy-and-swap)?

Share this post


Link to post
Share on other sites
Fruny    1658
Quote:
Original post by Zahlman
That sucks. Is there any good way to go about generalizing things then (i.e. create a templated assignment op using copy-and-swap)?


And why do you think I was working on a 'magic' swap function that would automatically detect and use if there was a member swap? [grin]

Share this post


Link to post
Share on other sites
petewood    819
Quote:
Original post by Fruny
Quote:
Original post by Zahlman
That sucks. Is there any good way to go about generalizing things then (i.e. create a templated assignment op using copy-and-swap)?


And why do you think I was working on a 'magic' swap function that would automatically detect and use if there was a member swap? [grin]


I was thinking about this. Would you come up with a traits class which could be specialised for classes which had a swap function:


//my_swap.h
//specialise this for types which have a member function 'swap'
template<typename Type>
struct SwapTraits {
static bool hasSwap = false;
};

template<bool useMember>
struct Swap {
template<typename Type>
static void DoIt(Type& lhs, Type& rhs) {
lhs.swap(rhs);
}
};

template<>
struct Swap<false> {
template<typename Type>
static void DoIt(Type& lhs, Type& rhs) {
std::swap(lhs, rhs);
}
};

template<typename Type>
void my_swap(Type& lhs, Type& rhs) {
Swap<SwapTraits<Type>::hasSwap>::DoIt(lhs, rhs);
}



usage:


#include "my_swap.h"
#include <vector>

//specialised for std::vector<double>
template<>
struct SwapTraits<std::vector<double> > {
static const bool hasSwap = true;
};

int main() {
int a(3),
b(5);
std::vector<double> c(20, 1.7),
d(45, 9.8);
my_swap(a, b);//uses std::swap
my_swap(c, d);//uses vector::swap
}



[Edited by - petewood on March 31, 2005 1:59:29 AM]

Share this post


Link to post
Share on other sites
mfawcett    373
Well, std::vector already specializes std::swap. It was mentioned earlier, but wouldn't the easiest thing be to just specialize std::swap on your type? That way the users can still just use std::swap like always, but it will just forward to your member swap.

Share this post


Link to post
Share on other sites
Fruny    1658
Quote:
Original post by petewood
I was thinking about this. Would you come up with a traits class which could be specialised for classes which had a swap function


We did. Ask Zahlman for the link.

Share this post


Link to post
Share on other sites
Zahlman    1682
I would have to search for it :P So here's the code again:

// introspect.h header with a bunch of evil macros

#ifndef INTROSPECT_H
#define INTROSPECT_H
typedef char (&n)[1];
typedef char (&y)[2];

template< bool condition, typename T> struct enable_if {};
template< typename T > struct enable_if<true,T> { typedef T type; };

#define INTROSPECT_IMPL(id, ret, func, cflag, ...) template< typename T, ret (T::*)(__VA_ARGS__) cflag> struct has_##id##_pfn {}; template< typename T > n has_##id##_x(...); template< typename T > y has_##id##_x(int, has_##id##_pfn<T, &T::func >* p = 0); template< typename T > struct has_##id { enum { value = sizeof(has_##id##_x<T>(0)) == sizeof(y) }; }

#define INTROSPECT(id, ret, func, ...) INTROSPECT_IMPL(id, ret, func,, __VA_ARGS__)
#define INTROSPECT_CONST(id, ret, func, ...) INTROSPECT_IMPL(id, ret, func, const, __VA_ARGS__)

#define WHEN_AVAILABLE(id, ret) template<typename T> typename enable_if<has_##id<T>::value,ret>::type

#define WHEN_UNAVAILABLE(id, ret) template<typename T> typename enable_if<!has_##id<T>::value,ret>::type

#define WHEN_AVAILABLE_INLINE(id, ret) template<typename T> inline typename enable_if<has_##id<T>::value,ret>::type

#define WHEN_UNAVAILABLE_INLINE(id, ret) template<typename T> inline typename enable_if<!has_##id<T>::value,ret>::type
#endif



// introspect.cpp with usage examples for the fast_swap case

#include <algorithm>
#include <vector>
#include <complex>
#include "introspect.h"

INTROSPECT(foo, void, swap, T);

WHEN_UNAVAILABLE_INLINE(foo, void) fast_swap(T& a, T& b) {
::std::swap<T>(a,b);
}

WHEN_AVAILABLE_INLINE(foo, void) fast_swap(T& a, T& b) {
a.swap(b);
}

int main() {
std::vector<int> vector1, vector2;
std::complex<int> complex1, complex2;
double a,b;

fast_swap(vector1, vector2);
fast_swap(complex1, complex2);
fast_swap(a,b);
}



// Example of using the introspection to wrap cout: oswrapper.h
// The operator << is defined for the wrapper class to use an object's
// "void operator() (oswrapper&)" when it exists, and the usual operator <<
// for cout otherwise.

#ifndef OSWRAPPER_H
#define OSWRAPPER_H

// I can't do this by extending ostream, because I wouldn't be able to make a
// wrapper representing cout...
struct oswrapper {
ostream& x;
oswrapper(ostream& x) : x(x) {}

// For some reason, the free-function operator<<'s won't pick up endl etc.
oswrapper& operator<<(ostream& ( *pf )(ostream&));
};

extern oswrapper zout;

#endif



// and oswrapper.cpp...

#include "introspect.h"
#include "oswrapper.h"
#include <iostream>

using namespace std;

oswrapper& oswrapper::operator<<(ostream& ( *pf )(ostream&)) {
x << pf;
return *this;
}

INTROSPECT_CONST(osf, void, operator(), oswrapper&);

WHEN_UNAVAILABLE(osf, oswrapper&) operator<<(oswrapper& os, const T& thing) {
os.x << thing;
return os;
}

WHEN_AVAILABLE(osf, oswrapper&) operator<<(oswrapper& os, const T& thing) {
thing(os);
return os;
}

oswrapper zout(cout);



However, this doesn't really help with implementing the assignment operator for classes without a swap method, does it? :s

IMHO this is the truly nice thing about GCd languages - because of not worrying so much about when you need to delete something, you can avoid certain questions about pointer ownership, and not have to worry about the Rule of Three or equivalent - and you can share objects more often, because you aren't worried about double-deletion. If you really need to copy something in a "every object is a reference" language (Java/Python/etc. object model), you make a new copy, and just throw the old thing away. :)

Share this post


Link to post
Share on other sites
petewood    819
Quote:
Original post by mfawcett
Well, std::vector already specializes std::swap. It was mentioned earlier, but wouldn't the easiest thing be to just specialize std::swap on your type? That way the users can still just use std::swap like always, but it will just forward to your member swap.


The point of my code is that to get it to use the specialised version you don't rewrite the function, you specialise the traits structure. I think that isn't as tedious or error prone. Which is good for me as I am prone to errors and need all the help I can get.

Zahlman: wow.

edit: removed 'std::swap isn't specialised for std::vector' because I was wrong.

[Edited by - petewood on March 31, 2005 3:05:36 PM]

Share this post


Link to post
Share on other sites
mfawcett    373
Quote:
Original post by petewood
Quote:
Original post by mfawcett
Well, std::vector already specializes std::swap. It was mentioned earlier, but wouldn't the easiest thing be to just specialize std::swap on your type? That way the users can still just use std::swap like always, but it will just forward to your member swap.


std::swap isn't specialised for std::vector.

The point of my code is that to get it to use the specialised version you don't rewrite the function, you specialise the traits structure. I think that isn't as tedious or error prone. Which is good for me as I am prone to errors and need all the help I can get.

Zahlman: wow.

You are probably right for your implementation. It is an implementation detail of Dinkumware I guess (not sure if any other STD library implentors did this because this is the only one I have installed). Here are the prototypes from my currently installed version:

void swap(vector& _Right);

friend void swap(vector<Type, Allocator >& _Left, vector<Type, Allocator >& Right);

I think I find this approach easier, although I'm in love with traits classes.

[Edited by - mfawcett on March 31, 2005 3:16:13 PM]

Share this post


Link to post
Share on other sites
dalleboy    324
Quote:
Original post by mfawcett
You are probably right for your implementation. It is an implementation detail of Dinkumware I guess (not sure if any other STD library implentators did this because this is the only one I have installed).

According to the C++ standard the following swap overloaded function exist in the namespace std.

namespace std {
template <class T, class Allocator>
void swap(vector<T,Allocator>& x, vector<T,Allocator>& y);
}

Share this post


Link to post
Share on other sites
petewood    819
Quote:
Original post by dalleboy
Quote:
Original post by mfawcett
You are probably right for your implementation. It is an implementation detail of Dinkumware I guess (not sure if any other STD library implentators did this because this is the only one I have installed).

According to the C++ standard the following swap overloaded function exist in the namespace std.


My bad. Sorry.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this