Avoid code duplication when using C++11 copy & move

Question

C++11 "move" is a nice feature, but I found it difficult to avoid code duplication (we all hate this) when used with "copy" at the same time. The following code is my implementation of a simple circular queue (incomplete), the two push() methods are almost the same except one line.

I have run into many similar situations like this. Any ideas how to avoid this kind of code duplication without using macro ?

=== EDIT ===

In this particular example, the duplicated code can be refactored out and put into a separate function, but sometimes this kind of refactoring is unavailable or cannot be easily implemented.

#include <cstdlib>
#include <utility>

template<typename T>
class CircularQueue {
public:
    CircularQueue(long size = 32) : size{size} {
        buffer = std::malloc(sizeof(T) * size);
    }

    ~CircularQueue();

    bool full() const {
        return counter.in - counter.out >= size;
    }

    bool empty() const {
        return counter.in == counter.out;
    }

    void push(T&& data) {
        if (full()) {
            throw Invalid{};
        }
        long offset = counter.in % size;
        new (buffer + offset) T{std::forward<T>(data)};
        ++counter.in;
    }

    void push(const T& data) {
        if (full()) {
            throw Invalid{};
        }
        long offset = counter.in % size;
        new (buffer + offset) T{data};
        ++counter.in;
    }

private:
    T* buffer;
    long size;
    struct {
        long in, out;
    } counter;
};

Answer 1

The simplest solution here is to make the parameter a forwarding reference. This way you can get away with only one function:

template <class U>
void push(U&& data) {
    if (full()) {
        throw Invalid{};
    }
    long offset = counter.in % size;
    // please note here we construct a T object (the class template)
    // from an U object (the function template)
    new (buffer + offset) T{std::forward<U>(data)};
    ++counter.in;
}

There are disadvantages with method though:

• it's not generic, that is it cannot always be done (in a trivial way). For instance when the parameter is not as simple as T (e.g.SomeType<T>).

• You delay the type check of the parameter. Long and seemingly unrelated compiler error may follow when push is called with wrong parameter type.

---

By the way, in your example T&& is not a forwarding reference. It's an rvalue reference. That's because T is not a template parameter of the function. It's of the class so it's already deduced when the class is instantiated. So the correct way to write your code would have been:

void push(T&& data) {
    ...
    ... T{std::move(data)};
    ...
}

void push(const T& data) {
   ... T{data};
   ...
}

Answer 2

The solution of using a forwarding reference is a good one. In some cases it gets difficult or annoying. As a first step, wrap it with an interface that takes explicit types, then in the cpp file send them to a template implementation.

Now sometimes that first step fails as well: if there are N different arguments that all need to be forwarded into a container, this requires an interface of size 2^N, and possibly it has to cross multiple layers of interfaces to get to the implementation.

To that end, instead of carrying or taking specific types, we can carry the end action(s) around. At the outermost interface, we convert arbitrary types into that/those action(s).

template<class T>
struct construct {
  T*(*action)(void* state,void* target)=nullptr;
  void* state=nullptr;
  construct()=default;
  construct(T&& t):
    action(
      [](void*src,void*targ)->T*{
        return new(targ) T( std::move(*static_cast<T*>(src)) );
      }
    ),
    state(std::addressof(t))
  {}
  construct(T const& t):
    action(
      [](void*src,void*targ)->T*{
        return new(targ) T( *static_cast<T const*>(src) );
      }
    ),
    state(const_cast<void*>(std::addressof(t)))
  {}
  T*operator()(void* target)&&{
    T* r = action(state,target);
    *this = {};
    return r;
  }
  explicit operator bool()const{return action;}
  construct(construct&&o):
    construct(o)
  {
    action=nullptr;
  }
  construct& operator=(construct&&o){
    *this = o;
    o.action = nullptr;
    return *this;
  }
private:
  construct(construct const&)=default;
  construct& operator=(construct const&)=default;
};

Once you have a construct<T> ctor object, you can construct an instance of T via std::move(ctor)(location), where location is a pointer properly aligned to store a T with enough storage.

A constructor<T> can be implicitly converted from a rvalue or lvalue T. It can be enhanced with emplace support as well, but that requires a bunch more boilerplate to do correctly (or more overhead to do easily).

Live example. The pattern is relatively simple type erasure. We store the operation in a function pointer, and the data in a void pointer, and reconstruct the data from the void pointer in the stored action function pointer.

There is modest cost in the above type erasure/runtime concepts technique.

We can also implement it like this:

template<class T>
struct construct :
  private std::function< T*(void*) >
{
  using base = std::function< T*(void*) >;
  construct() = default;
  construct(T&& t):base(
    [&](void* target)mutable ->T* {
      return new(target) T(std::move(t));
    }
  ) {}
  construct(T const& t):base(
    [&](void* target)->T* {
      return new(target) T(t);
    }
  ) {}
  T* operator()(void* target)&&{
    T* r = base::operator()(target);
    (base&)(*this)={};
    return r;
  }
  explicit operator bool()const{
    return (bool)static_cast<base const&>(*this);
  }
};

which relies on std::function doing the type erasure for us.

As this is designed to only work once (we move from the source), I force an rvalue context and eliminate my state. I also hide the fact I'm a std::function, because it doesn't follow those rules.