c++c++11copy-constructorassignment-operatorcopy-and-swap

Implementing the swap in the copy and swap idiom


Following What is the copy and swap idiom and How to provide a swap function for my class, I tried implementing the swap function like in the latter accepted answer option number 2 (having a free function that calls a member function) instead of the direct friendly free function in the former link.

However the following doesn't compile

#include <iostream>

// Uncommenting the following two lines won't change the state of affairs
// class Bar;
// void swap(Bar &, Bar &);
class Bar {
public:
  Bar(unsigned int bottles=0) : bottles(bottles) { enforce(); } // (1)
  Bar(Bar const & b) : bottles(b.bottles) { enforce(); } // (1)

  Bar & operator=(Bar const & b) {
    // bottles = b.bottles;
    // enforce();
    // Copy and swap idiom (maybe overkill in this example)
    Bar tmp(b); // but apart from resource management it allows (1)
                // to enforce a constraint on the internal state
    swap(*this, tmp); // Can't see the swap non-member function (2)
    return *this;
  }

  void swap(Bar & that) {
    using std::swap;
    swap(bottles, that.bottles);
  }

  friend std::ostream & operator<<(std::ostream & out, Bar const & b) {
    out << b.bottles << " bottles";
    return out;
  }

private:
  unsigned int bottles;
  void enforce() { bottles /=2; bottles *= 2; } // (1) -- Ensure the number of bottles is even
};

void swap(Bar & man, Bar & woman) { // (2)
  man.swap(woman);
}

int main () {
  Bar man (5);
  Bar woman;

  std::cout << "Before -> m: " << man << " / w: " << woman << std::endl;
  swap(man, woman);
  std::cout << "After  -> m: " << man << " / w: " << woman << std::endl;

  return 0;
}

I know that the copy and swap idiom is overkill here but it also allows one to enforce some constraint on the internal state through the copy constructor (1) (A more concrete example would be to maintain a fraction in reduced form). Unfortunately, this doesn't compile because the only candidate for (2) that the compiler sees is the Bar::swap member function. Am I stuck with the friend non-member function approach?

EDIT: Go to my answer below to see what I ended up with thanks to all the answers and comments on this question.


Solution

  • I take it we're post c++11?

    In which case, the default implementation of std::swap will be optimal, provided we correctly implement the move-assignment operator and the move-constructor (ideally nothrow)

    http://en.cppreference.com/w/cpp/algorithm/swap

    #include <iostream>
    
    class Bar {
    public:
        Bar(unsigned int bottles=0) : bottles(bottles) { enforce(); } // (1)
        Bar(Bar const & b) : bottles(b.bottles) {
            // b has already been enforced. is enforce necessary here?
            enforce();
        } // (1)
        Bar(Bar&& b) noexcept
        : bottles(std::move(b.bottles))
        {
            // no need to enforce() because b will have already been enforced;
        }
    
        Bar& operator=(Bar&& b) noexcept
        {
            auto tmp = std::move(b);
            swap(tmp);
            return *this;
        }
    
        Bar & operator=(Bar const & b)
        {
            Bar tmp(b); // but apart from resource management it allows (1)
            swap(tmp);
            return *this;
        }
    
        void swap(Bar & that) noexcept {
            using std::swap;
            swap(bottles, that.bottles);
        }
    
        friend std::ostream & operator<<(std::ostream & out, Bar const & b) {
            out << b.bottles << " bottles";
            return out;
        }
    
    private:
        unsigned int bottles;
        void enforce() {  } // (1)
    };
    
    /* not needed anymore
    void swap(Bar & man, Bar & woman) { // (2)
        man.swap(woman);
    }
    */
    int main () {
        Bar man (5);
        Bar woman;
    
        std::cout << "Before -> m: " << man << " / w: " << woman << std::endl;
        using std::swap;
        swap(man, woman);
        std::cout << "After  -> m: " << man << " / w: " << woman << std::endl;
    
        return 0;
    }
    

    expected result:

    Before -> m: 5 bottles / w: 0 bottles
    After  -> m: 0 bottles / w: 5 bottles
    

    EDIT:

    For the benefit of anyone who is concerned about performance (e.g. @JosephThompson) allow me to allay your concerns. After moving the call to std::swap into a virtual function (to force clang to produce any code at all) and then compiling with apple clang with -O2, this:

    void doit(Bar& l, Bar& r) override {
        std::swap(l, r);
    }
    

    became this:

    __ZN8swapper24doitER3BarS1_:            ## @_ZN8swapper24doitER3BarS1_
        .cfi_startproc
    ## BB#0:
        pushq   %rbp
    Ltmp85:
        .cfi_def_cfa_offset 16
    Ltmp86:
        .cfi_offset %rbp, -16
        movq    %rsp, %rbp
    Ltmp87:
        .cfi_def_cfa_register %rbp
        movl    (%rsi), %eax
        movl    (%rdx), %ecx
        movl    %ecx, (%rsi)
        movl    %eax, (%rdx)
        popq    %rbp
        retq
        .cfi_endproc 
    

    See? optimal. The c++ standard library rocks!