Back to General and Gameplay Programming

copying classes

General and Gameplay Programming Programming

Started by Lode March 29, 2005 02:56 AM

24 comments, last by mfawcett 19 years ago

374

March 29, 2005 04:55 PM

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);

--Michael Fawcett

ChaosEngine

5,385

March 29, 2005 05:35 PM

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.

if you think programming is like sex, you probably haven't done much of either.-------------- - capn_midnight

Fruny

1,658

March 29, 2005 05:37 PM

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.

"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan

ChaosEngine

5,385

March 29, 2005 06:01 PM

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!

if you think programming is like sex, you probably haven't done much of either.-------------- - capn_midnight

Deyja

920

March 29, 2005 07:39 PM

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]

http://jemgine.omnisu.com

Zahlman

1,682

March 30, 2005 12:17 AM

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)?

Fruny

1,658

March 30, 2005 08:21 AM

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]

Qw3r7yU10p!

819

March 30, 2005 11:59 AM

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]

mfawcett

374

March 30, 2005 06:17 PM

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.

--Michael Fawcett

Fruny

1,658

March 30, 2005 07:10 PM

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.

copying classes

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

copying classes

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines