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};
   ...
}