I would like to design an elegant yet efficient circular buffer of integers.
I have designed a FIFO type circular buffer in C++(11)- starting with the class header (declaration) as follow:

[cyclicBuffer.hpp]:

#ifndef __CyclicBuff_HPP

#define __CyclicBuff_HPP

#pragma once



class CyclicBuffer

{

public:

    CyclicBuffer(int buffSize);

    ~CyclicBuffer();

    void setChar(char c);

    char getChar();

    void clearCycBuff(int buffSize);

private:

    char *buffer;

    int bufferSize;

    int head;   // index for the top of the buffer

    int tail;   // indes for the bottom of the buffer

};



#endif  

implementation of the class is as following:
[cyclicBuffer.cpp]:

#include "stdafx.h"

#include "cyclicBuffer.hpp"

#include <iostream>



//using namespace std;



CyclicBuffer::CyclicBuffer(int buffSize)

{

    head = 0;

    tail = 0;

    bufferSize = buffSize;

    buffer = new char[bufferSize];

    std::cout << "Buffer initated successfullyn";



/*

    {

        std::cout << "Buffer was not createdn driver exiting...n";

//      return -1;

    }*/

}



CyclicBuffer::~CyclicBuffer()

{

    delete buffer;

    std::cout << "Buffer destroyed successfullyn";

}





void CyclicBuffer::clearCycBuff(int buffSize)   //not used , has no meaning to set '0'



{

    head = 0;

    tail = 0;

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

        buffer[i] = '0';

}



void CyclicBuffer::setChar(char c)

{

    //std::cout << "head: " << head << 'n';

    head %= bufferSize;

    buffer[head] = c;

    head++;

}



char CyclicBuffer::getChar(void)

{

    //std::cout << " tail:" << tail<<'n';

    tail %= bufferSize;

    return buffer[tail++];



}

Finally, testing the class by an instance as follow:

// CyclicBufferMain.cpp : Defines the entry point for the console 

// application.





#include "stdafx.h"

#include <iostream>

#include "cyclicBuffer.hpp"



#define SIZE    32



int main(void)

{

    CyclicBuffer aBuffer(SIZE);



    for (int i = 0; i < SIZE*1.5; i++)

        aBuffer.setChar((char)(i+45));



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

        std::cout << aBuffer.getChar() << ' ';





    std::cout << 'n';







    return 0;

}

for the given main function the result will be like this:

My question: is this class is efficient enough for low-rate uC ? (targeting to Atmel, RISC type or low performance ARM uC)

Regarding @Toby Speight: Thanks for your helpful comments and advices - few questions for you:

(1) when I did delete instead delete , did I make a 'memory leak'? why MS VStudio (2015) didn't warn me?

(2) I would change the CyclicBuffer::clearCycBuff() member function to use buffer[i] = 0; (instead buffer[i]='0';) to clear the buffer's values.

(3) Why need to drop the commented-out code?

(4) You used uniform initializer at the constructor - is it big difference as to what I did using regular variable assignments at the constructor ?

(5) I know 'size_t' is it a specialize typedef that exists at the GNU and MS compilers but would have a problem using it when porting the code to a specific target which could not have such typedef ?

Thanks.

Improve the wrap-around logic

There's obvious inefficiency in:

head %= bufferSize;

% uses division, but because we're incrementing, we know that head / bufferSize is at most 1. Instead, we can:

if (++head >= bufferSize)

    head -= bufferSize;

On architectures such as ARM, there's no branch here, as any decent compiler will just use a condition flag on the subtraction.

Consider checking for overrun and underrun

At the moment, this buffer has no checking for the read and write positions crossing each other. Perhaps that's what you need, but it's quite unusual to have an unchecked circular buffer, and if you were holding (e.g.) audio data, you can imagine how it would sound, given the output you show.

Match new and delete

Since you create buffer using new, you must use the array form delete in the destructor:

CyclicBuffer::~CyclicBuffer()

{

    delete buffer;

}

Style points

Prefer initializers to assignment

A constructor should initialize all the members of the class:

CyclicBuffer::CyclicBuffer(std::size_t buffSize)

  : buffer(new char[bufferSize]),

    bufferSize(buffSize),

    head(0),

    tail(0)

{

}

A good compiler will warn you if you have uninitialized members (e.g. g++ -Wall -Wextra).

Note that I've also changed the argument to be a size_t, as a negative size is meaningless. The int members should all be size_t, too.

Avoid numeric character constants

What's special about the number 45? If you care about its character value, (and if you're using an ASCII-like system), it's clearer to write '-' instead.

Drop the commented-out code

I don't want to see using std::namespace;, ever. Not even in a comment.

Constructor initialization - 0 out the array

buffer(new T[size]{}) will set all the values to 0 instead of random garbage

template<class T>

CyclicBuffer<T>::CyclicBuffer(std::size_t size): head(0), tail(0), buffer(new T[size]{}), size(size) {



}

Adding and Getting can use equality comparison

Comparing equality rather than subtracting/greater or equals makes it more readable

template<class T>

void CyclicBuffer<T>::add(T val) {

    if (head == size)

        head = 0;



    buffer[head++] = val;

}



template<class T>

T CyclicBuffer<T>::get() {

    if (tail == size)

        tail = 0;



    return buffer[tail++];

}

Use template to make your structure generic

You will be able to insert any type:

CyclicBuffer<char> charCyclicBuffer(5);

CyclicBuffer<int> intCyclicBuffer(5);

Use std::size_t for portability

Besides portability - size, head, tail will never be negative

template<class T>

class CyclicBuffer{

public:

    explicit CyclicBuffer(std::size_t size);



    ~CyclicBuffer();



    void add(T val);



    T get();



    inline std::size_t getSize() {

        return this->size;

    }



private:

    T *buffer;

    std::size_t size;

    std::size_t head;

    std::size_t tail;

};