Implementation of fixed size queue using a ring (cyclic) buffer

I found myself in need of a fixed size queue and decided to implement one using a ring (cyclic) buffer.

I have tried my best to match the API of std::queue with the addition of full() to test if the queue is full and unable to accept another element.

The code compiles cleanly with: -Wall -Wextra -pedantic --std=c++14 -lgtest -lgtest_main, it runs and all tests pass on clang 3.9.1. Unfortunately at least GCC 4.9.4 and below cannot compile the header file due to a bug where a noexcept specification can't refer to a member.

All comments welcome.

File: xtd/fixed_queue.hpp

```
#ifndef GUARD_INCLUDE_XTD_FIXED_QUEUE_HPP
#define GUARD_INCLUDE_XTD_FIXED_QUEUE_HPP

#include
#include
#include

namespace xtd {

template
class fixed_queue {
public:
using value_type = T;
using reference = value_type&;
using const_reference = const value_type&;
using size_type = std::size_t;

fixed_queue() = default;
fixed_queue(const fixed_queue& other) { *this = other; }
fixed_queue(fixed_queue&& other) { *this = std::move(other); }
~fixed_queue() { clear(); }

fixed_queue& operator=(const fixed_queue& other) {
clear();

auto i = other.m_read_idx;
while (i != other.m_write_idx) {
emplace(*other.get(i));
i = other.increment_index(i);
}
return *this;
}

fixed_queue& operator=(fixed_queue&& other) {
clear();
while (!other.empty()) {
emplace(std::move(other.front()));
other.pop();
}
return *this;
}

size_type capacity() const { return N; }

size_type size() const {
if (empty()) {
return 0;
} else if (m_write_idx > m_read_idx) {
return m_write_idx - m_read_idx;
} else {
return N - m_read_idx + m_write_idx + 1;
}
}

void clear() {
while (!empty()) {
pop();
}
}

bool full() const { return size() == capacity(); }

bool empty() const { return m_write_i

Non Standard Copy Semantics

Interested in why you chose to implement the copy constructor in terms of the copy assignment operator and not the other way around. Noting that the standard way to implement this is the copy and swap idiom (other way around).

I can see this is probably slightly more efficient. But is the decrease in readability worth it. Only you can answer that.

fixed_queue(const fixed_queue& other) { *this = other; }
fixed_queue& operator=(const fixed_queue& other) {
  clear();

  auto i = other.m_read_idx;
  while (i != other.m_write_idx) {
    emplace(*other.get(i));
    i = other.increment_index(i);
  }
  return *this;
}

The one issue I have with this is that it does not provide the strong exception guarantee and you can't fall back to the original state if something goes wrong.

Though this is correct. I don't like the copy constructor not explicitly initializing the members. Have to go check the rest of the code to make sure the members are initialized reeks of doing things in multiple places.

Move Semantics

The source of the move can be left in an undefined state (as long as it is valid).

I don't see the need to pop() values from the source object. just move them. Poping them adds extra work that is not required. The destructor when called when do all the cleanup required.

fixed_queue& operator=(fixed_queue&& other) {
  clear();
  while (!other.empty()) {
    emplace(std::move(other.front()));

    // Don't need this.
    // Though if you do remove this you need to change the above line
    // to get a reference to an internal member.
    other.pop();
  }
  return *this;
}

Alignment

Not 100% convinced this works.

alignas(value_type) std::array m_data;

You want the internals of the array (ie. the array members) to be aligned to value_type. This is technically aligning the array (not its members). I have not read up on the requirements of the array (so this may work) but this seems a bit doggy.

Just point this works because std::array is a special case (guaranteed to only have one element that aligns with the specified data type). In general aligning containers on their content data type may not work.

Initialization

size_type m_write_idx = 0;
size_type m_read_idx = 0;

I prefer the constructor to initialize members. It's easier to spot when things are missed. If you are going to initialize the members in the code then I prefer them near the construcors so that it is easy to spot.

Personally I lay my code out like this:

Class
    private   Variables
    public    Constructors/Assignment/Destructors

    public    methods
    protected methods
    private   methods

This way I can quickly see if the members have all been initialized.

StackExchange Code Review Q#160360, answer score: 4