I am learning circular / cyclic buffer using this link. Here is the code that I have written so far:

#include <iostream>

#include <memory>

#include <cstddef>



class CircularBuffer

{

    std::unique_ptr<int> u_ptr;

    std::size_t head;

    std::size_t tail;

    const std::size_t capacity;

    bool full_v;

public:

    CircularBuffer(std::size_t);

    void reset();

    bool full() const;

    bool empty() const;

    std::size_t size() const;

    void write(int);

    int read();

};



// Constructor

// Set the capacity via initializer list because it is const

CircularBuffer::CircularBuffer(std::size_t space):capacity{space}

{

    u_ptr = std::unique_ptr<int>(new int[space]);

    head = 0;

    tail = 0;

    full_v = false;

}



// Reset the Buffer

void CircularBuffer::reset()

{

    head = 0;

    tail = 0;

    full_v = false;

    u_ptr[head] = int{};

}



// Check if buffer is full

bool CircularBuffer::full() const

{

    return full_v;

}



// Check if buffer is empty

bool CircularBuffer::empty() const

{

    return (!full_v && head == tail);

}



// Return the size of the buffer

std::size_t CircularBuffer::size() const

{

    if(full_v)

    {

        return capacity;

    }

    if(head >= tail)

    {

        return head - tail;

    }

    else

    {

        return capacity - (tail - head);

    }

}



// Write values into the buffer

void CircularBuffer::write(int data)

{

    u_ptr[head] = data;

    head = (head + 1) % capacity;

    if(full_v)

    {

        tail = (tail + 1) % capacity;

    }

    full_v = (head == tail);

}



// Read from buffer

int CircularBuffer::read()

{

    if(this -> empty())

    {

        return int{};

    }

    int ret_val = u_ptr[tail];

    full_v = false;

    tail = (tail + 1) % capacity;

    return ret_val;

}



int main()

{

    CircularBuffer cb{10};

    std::cout << "Empty: " << cb.empty() << "n";

    for(int i = 0; i < 10; i++)

    {

        cb.write(i);

    }

    std::cout << "Full: " << cb.full() << "n";

    std::cout << "Read: " << cb.read() << "n";

    std::cout << "Full: " << cb.full() << "n";

    std::cout << "Empty: " << cb.empty() << "n";

    std::cout << "Size: " << cb.size() << "n";

    cb.write(35);

    std::cout << "Size: " << cb.size() << "n";

    std::cout << "Read: " << cb.read() << "n";

    std::cout << "Size: " << cb.size() << "n";

    cb.reset();

    std::cout << "Size: " << cb.size() << "n";

    return 0;

}

1. Is the implementation correct? What are the shortcomings that can be fixed?




2. I see that the original reference uses a mutex object and uses lock. Is such an approach used with all data structures? If not, is there a reason why that has been used here?

Your ringbuffer has a compile time size, so it would be appropriate to make it a template class rather than simply passing it as an argument to the constructor. You never plan to change it anyway?

template<size_t bufSize>

class CircularBuffer

That leads to the next point. You use a std::unique_ptr<int> There is something in C++ that is much better than this std::array. With the size of the buffer a compile time template argument of the class you can easily use it

template<size_t bufSize>

class CircularBuffer {

    std::array<int, bufSize> buf;

    std::size_t head;

    std::size_t tail;

    bool full_v;



    ...

};

Note that this increases the size of the class object considerably, as the std::array is directly inlined in the class.

The other members of this class are always the same after construction so you should use static member initialization:

template<size_t bufSize>

class CircularBuffer {

    std::array<int, bufSize> buf;

    std::size_t head{ 0 };

    std::size_t tail{ 0 };

    bool full_v{ false };



    ...

};

Note that this completely removes the need to define a constructor at all. The compiler generated default constructor will do just fine.

Resetting the buffer should not change the data stored in it as it is not read anyway so just leave it alone

void CircularBuffer::reset()

{

    head = 0;

    tail = 0;

    full_v = false;

}

Can tail ever be larger than head? If not why are you checking it.

Correctness and design

I will skip all points miscco
already raised and go directly to void write and int read.

• Your write overwrites old values when the buffer is full.


• Your read returns zero if the buffer is empty.

Although understandable, it should at least be documented:

/// Describe the class, what is the purpose, what is important

template<class T, size_t N> // see std::array

class CircularBuffer {

...

    /// Write values into the buffer, overwrite old values when the buffer is full

    void write(const T& data) {

or maybe:

    /// Write values into the buffer

    ///return true if written, false if buffer was full

    bool write(const T& data) {

You can also notice that I made it a template, just like std::array - both the type and the capacity are now template parameters. The standard way of passing arguments to methods like write is to use const T&, but it has some drawbacks, so, if it trully is for embedded systems, then simple T is an option as well (knowing you are going to use it with integers and such). But document it (and/or add other template parameters with some defaults).

Synchronization (Locking)

If you are going to use it with multiple threads (or main loop vs. interrupt) then some form of synchronization is indeed needed, because there are possible race-conditions otherwise, like one thread being inside write, just overwriting the oldest slot, while another thread can be inside read getting this newest element, while it should get the oldest, but tail was not yet updated by the first thread.

First question to ask is: How many threads / concurrent readers and writers (producers and consumers)?
Generic solution could use mutex but there can be better solutions for single producer single consumer (e.g. main loop vs. interrupt - USART and similar communication synchronization). One possibility is to use separate (atomic) write and read counters (like head and tail but you always only increase the counters and substract them to get number of elements inside the buffer - but be careful with order of operations, it is not trivial).