Simple ring/circular buffer c++ class V2

Question

The Original Post (v1)

I'm looking for feedback on the updated version of the code posted at the link above.

#pragma once

#include <memory>
#include <cassert>
#include <type_traits>

namespace datastructures {

    template<class _Ty, size_t _Size>
    class CircularBufferv2 {

        // uses char as type to prevent default _Ty initialization.
        alignas(alignof(_Ty)) char buffer[_Size * sizeof(_Ty)];
        size_t head;
        size_t tail;
        bool isFull;

    public:
        constexpr CircularBufferv2() noexcept :
            buffer{0},
            head{0},
            tail{0},
            isFull{false} {
        }

        void push(const _Ty& item) noexcept {
            assert(!isFull && "Attempting to push item into full buffer!");

            new (&buffer[head * sizeof(_Ty)]) _Ty(std::move(item));

            head = ++head % _Size;
            isFull = head == tail;
        }

        _Ty pop() noexcept {
            assert(!is_empty() && "Attempting to pop item from empty buffer!");

            auto location = reinterpret_cast<_Ty*>(&buffer[tail * sizeof(_Ty)]);
            auto result = std::move(*location);
            std::destroy_at(location);

            tail = ++tail % _Size;
            isFull = false;

            return result;
        }

        _NODISCARD constexpr _Ty& peek() noexcept {
            assert(!is_empty() && "Attempting to peek in empty buffer!");

            return *reinterpret_cast<_Ty*>(&buffer[tail * sizeof(_Ty)]);
        }

        _NODISCARD constexpr const _Ty& peek() const noexcept {
            assert(!is_empty() && "Attempting to peek in empty buffer!");

            return *reinterpret_cast<_Ty*>(&buffer[tail * sizeof(_Ty)]);
        }

        _NODISCARD constexpr bool is_empty() const noexcept {
            return !isFull && tail == head;
        }

        _NODISCARD constexpr size_t get_capacity() const noexcept {
            return _Size;
        }

        _NODISCARD constexpr size_t get_size() const noexcept {
            if (isFull)
                return _Size;

            if (head >= tail)
                return head - tail;

            return _Size + head - tail;
        }

        _NODISCARD _CONSTEXPR17 _Ty* data() noexcept {
            return buffer;
        }

        _NODISCARD _CONSTEXPR17 const _Ty* data() const noexcept {
            return buffer;
        }
    };
}

I want to take advantage of all of the new features(c++17) while also supporting older compilers(preferably all older compilers but c++11 is probably as old as i'll actually be compiling for). Any suggestions welcome. (I'm attempting to use this class as an example class to follow when constructing other classes.)

Also, the use of _CONSTEXPR17 on the data functions I was wondering why use the macro vs just a constexpr? (I based the use of macros around the std::array struct, its data function uses _CONSTEXPR17 rather than just constexpr.)

Answer 1

Instead of a char array you can use std::aligned_storage.

You aren't consistent with your macros you have both constexpr and _CONSTEXPR17. Pick one and stick to it.

destroy_at is standard only from C++17 onward. The equivalent syntax is location->~_Ty(); However it may be worth to put this and the placement new into a helper function to move the ugly syntax elsewhere and clarify the function.

You don't have a move variant of push or a bulk add/remove using iterators.

Answer 2

Is there any reason for your users to access your internal buffer? To me that data methods shouldn't be there.

Don't like very much those reinterpret_cast but I see you have those there to support not default constructable types. I could live with those and get back that additional feature as long as that implementation detail is sealed into the class.