C++ Circular buffer through Circular iterator

Question

I saw some question on SO lately involving circular buffer, like a chain of descriptors for data transfer. All solutions involved lots of lines.

I wanted an implementation with as few lines as possible, as inspired by this answer. The idea is not to have a circular buffer but a usual container and a specific iterator able to wrap.

Question: Is it possible to achieve this with as few lines as in the example below or are there hidden pitfalls ?

• This code was intended to spin forever on a container which size do not change, more information about iterator validity under containers changes

Additional information after first answers / comments

Similar 2009 question on SO mentioned by Mercury Dime (the question uses the term cyclic or circulator and not circular)

EDIT: Corrections of the more evident errors / typos based on incomputable's answer

#include <iostream>
#include <list>

template <class BaseIter>
class CircularIterator:public BaseIter {
        //inspired from https://stackoverflow.com/a/947754/3972710
        BaseIter begin,end;
    public:
        CircularIterator(BaseIter b, BaseIter e ):BaseIter(b), begin(b), end(e) {}
        CircularIterator & operator ++(void)
        {
            BaseIter::operator++();
            if(*this == end)
                BaseIter::operator=(begin);
            return *this;
        }
        const CircularIterator   operator ++(int)
        {
            const auto oldValue = *this;
            this->operator++();
            return oldValue;
        };

        CircularIterator & operator --(void) = delete;
        const CircularIterator   operator --(int)  = delete;
};

int main()
{
    std::list<int> intList = { 1, 2, 3};
    CircularIterator<std::list<int>::iterator> circIter(intList.begin(),intList.end());

    auto it = circIter++;

    std::cout << *(it++) << "\n";
    std::cout << *(it++) << "\n";
    std::cout << *(it++) << "\n";
    std::cout << *(it++) << "\n";
    std::cout << "..." << "\n";
    std::cout << "Hello world!\n";
    return 0;
}

Original Code

#include <iostream>
#include <list>
using namespace std;
//C++11 at least

template <class baseIter>
class circularIterator:public baseIter {
        //inspired from https://stackoverflow.com/a/947754/3972710
        baseIter begin,end;
    public:
        circularIterator(baseIter b, baseIter e ):baseIter(b), begin(b), end(e) {}
        baseIter & operator ++(void) {  baseIter::operator++();
                                        if(*this == end) baseIter::operator=(begin);
                                        return *this;}
        baseIter & operator ++(int) = delete;
};

int main()
{
    list<int> intList = { 1, 2, 3};
    circularIterator<list<int>::iterator> circIter(intList.begin(),intList.end());

    cout << *circIter << endl;
    cout << *(++circIter) << endl;
    cout << *(++circIter) << endl;
    cout << *(++circIter) << endl;
    cout << *(++circIter) << endl;
    cout << "..." << endl;

    cout << "Hello world!" << endl;
    return 0;
}

Answer 1

High level overview:

The idea is solid, but creating an iterator adapter to make the container behave differently just sounds weird. It would be better to have container adapters, as std::stack<> and std::queue<> do.

Implementation overview:

The code has a serious issues. Mainly the following:

• It has a chance to break on some algorithms:

   auto second = ++first;
  

where first is circularIterator<> will make second to be baseIter. The most dangerous ones are sorting algorithms, especially those that use divide and conquer. The template type parameter is usually only one, e.g. only one type of iterators are allowed, as a result the compiler will issue substitution failure error.

• Will not work on idiomatic while (*first++ != last) or similar.

The postfix increment is deleted, thus it disables probably most of the standard library.

• The class pretends to be an iterator, but is a range.

Usually iterator should know about its own internal state only. Knowing end iterator should be last resort. Actually, the reason for storing the range is because the code doesn't provide a function to create a pair of circularIterators. If such function would be provided, the operator==() of parent iterator would still work, as they mostly take it by reference.

Cosmetics:

Not really important, but here they are:

• Usually people name types (and classes, as a result) in PascalCase or snake_case. Starting with a lowercase letter, camelCase is used for variable names.

• Indentation and brace placing is very weird.

Misc:

• using namespace std; is bad. It causes name clashes and in general confuses other mechanisms of C++.

• std::endl is a performance drainer. If done in a tight loop, can cause serious issues. Bjarne Stroustrup himself didn't use it for a decade.

Answer 2

Don't write (void) where you mean (). Stroustrup calls this "an abomination" and it has no purpose in C++. It was necessary in ANSI C in order to adopt C++ declaration syntax while still retaining the old style, but in C++ an empty parameter list means no parameters, pure and simple.