the most safe way to swap two class instance

Question

Use swap to swap two class instances, sometimes it might throw errors.

#include <iostream>
#include <string>
using namespace std;

#include <cstring>
class Buffer {
public:
    Buffer(const string& s): buffer(s) {}
    string buffer;
};

template<class _Tx>
void SWAP(_Tx& a, _Tx& b) {
    size_t size = sizeof(_Tx);
    char buffer[size];
    memcpy(buffer, &a, size);
    memcpy(&a, &b, size);
    memcpy(&b, buffer, size);
}

int main() {
    Buffer a("This is a"), b("This is b");
    swap(a,b);
    cout << a.buffer << endl;

    SWAP(a,b);
    cout << b.buffer << endl;
    return 0;
}

the std::swap will do something like this:

template<class _Tx>
void swap(_Tx &a, _Tx &b) {
    _Tx t = a;
    a = b;
    b = t;
}

_Tx t = a; will call the copy constructor of _Tx, which is Buffer::Buffer(Buffer &e) in this case. This method try to allocate some memory, and this might cause some errors.

I try to use another method instead of std::swap:

template<class _Tx>
void SWAP(_Tx& a, _Tx& b) {
    char buffer[sizeof(_Tx)];
    memcpy(buffer, &a, sizeof(_Tx));
    memcpy(&a, &b, sizeof(_Tx));
    memcpy(&b, buffer, sizeof(_Tx));
}

Is it a safe way ???

UPDATES std::swap might be safe with c++0x. This is the comparison: c99 vs. c++0x

REF what-is-the-copy-and-swap-idiom Thanks Donal Fellows's remind

Of course using `memcpy` on non-trivial types is much safer than goold old `std::swap` (I hope you won't mind the sarcasm ;)). — Christian Rau, Nov 15 '12 at 10:47
possible duplicate of [What is the copy-and-swap idiom?](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) — Donal Fellows, Nov 15 '12 at 11:50

billz · Answer 1 · 2012-11-15T09:06:55.613

2

The problem is with the dynamic allocated pointer buffer, why don't you use std::string instead

copy constructor signature is:

Buffer(const Buffer &e)

now you swap the objects:

int main(int agrc, char* argv[])
{
  Buffer a("This is a"), b("This is b");
  std::swap(a,b);
}

std::swap code shall be faster than your SWAP code

template<class _Ty> inline
void swap(_Ty& _Left, _Ty& _Right)
{   // exchange values stored at _Left and _Right
  _Ty _Tmp = _Move(_Left);
  _Left = _Move(_Right);
  _Right = _Move(_Tmp);
}

edited Nov 15 '12 at 09:06

answered Nov 15 '12 at 08:42

billz

44,644
9
83
100

I don't think it's the point. If `string` is embedded in the `Buffer` , the `std::swap(Buffer &, Buffer &)` will not use the specialized `string::swap` to avoid reallocation of the `string` field, so there still might be a problem. – xiaoyi Nov 15 '12 at 08:47
1

@idy you don't need your SWAP anymore, just use std::swap – billz Nov 15 '12 at 08:59
@billz but `std::swap` will call the `string(const string& str)` to do the assignment. The string's assignment method will dynamic alloc memory. – idy Nov 15 '12 at 09:05
I think std::swap is faster than you SWAP? – billz Nov 15 '12 at 09:07
@billz It dependences on the member's assignment constructor??? `std::swap` need three times of assignment. – idy Nov 15 '12 at 09:20
1

it's using move semantics, which only moves the pointer around. unless I am wrong. – billz Nov 15 '12 at 09:22
@billz [sample@ideone](http://ideone.com/Jo6vOH) `std::swap` actually called the assignment constructor of `Inner` class. – idy Nov 15 '12 at 09:32
Not if you define `Buffer& operator=(Buffer&& b)` – billz Nov 15 '12 at 09:38
@idy Of course using const rvlaue references is pretty useless and in fact doesn't give you a move constructor at all. And I guess the compiler used by ideone is not good at automatically generating move members. – Christian Rau Nov 15 '12 at 10:52

score 0 · Answer 2 · answered Nov 15 '12 at 08:48

0

It will work for this particular Buffer object but may not be safe at all if it evolves of if you use it with another class. It's the role of the assignment operator and/or copy constructor of the class to make it safe.

answered Nov 15 '12 at 08:48

pagra

665
4
11

the most safe way to swap two class instance

2 Answers2