5
\$\begingroup\$

Python has that range:

>>> range(10, -20, -3)
[10, 7, 4, 1, -2, -5, -8, -11, -14, -17]

I have this C++ implementation of exactly the same facility:

range.h:

/* 
 * File:    range.h
 * Author:  Rodion "rodde" Efremov
 * Version: (Nov 30, 2015)
 */

#ifndef RANGE_H
#define RANGE_H
#include <stdexcept>

namespace coderodde {

    template<typename Int = int>
    class range {
    private:
        Int m_start;
        Int m_end;
        Int m_step;

        class range_iterator {
        private:
            Int    m_value;
            Int    m_step;
            size_t m_count;

        public:
            range_iterator(Int value, 
                           Int step, 
                           size_t count) : m_value{value}, 
                                           m_step{step},
                                           m_count{count} {}

            int  operator*() { return m_value; }
            bool operator!=(range_iterator& other) { return m_count != other.m_count; }
            void operator++() { m_value += m_step; ++m_count; }
        };

    public:
        range(Int start, Int end, Int step) : m_start{start},
                                              m_end{end},
                                              m_step{step} {}

        range(Int start, Int end) : range(start, end, 1) {}

        range_iterator begin() { return range_iterator(m_start, m_step, 0); }

        range_iterator end() { 
            if (m_step == 0) throw std::runtime_error("The step is zero.");

            if (m_start <= m_end) 
            {
                if (m_step < 0) 
                {
                    return range_iterator(0, 0, 0);
                }

                return range_iterator(m_start, m_step, 
                                     (m_end - m_start) / m_step + 
                                   (((m_end - m_start) % m_step) ? 1 : 0));
            }
            else 
            {
                if (m_step > 0) 
                {
                    return range_iterator(0, 0, 0);
                }

                m_step = -m_step;
                return range_iterator(m_start, m_step, 
                                     (m_start - m_end) / m_step +
                                   (((m_start - m_end) % m_step) ? 1 : 0));
            }
        } 
    };
}

#endif  /* RANGE_H */

With demo main.cpp:

#include <iostream>
#include "range.h"

using std::cin;
using std::cout;
using std::endl;
using coderodde::range;

int main(int argc, char* argv[]) 
{
    while (true)
    {
        int start = 0;
        int end   = 0;
        int step  = 0;

        cout << ">> ";

        if (not (cin >> start >> end >> step)) 
        {
            break;
        }

        try 
        {
            for (auto i : range<>(start, end, step)) 
            {
                cout << i << endl;
            }
        } 
        catch (std::runtime_error& err)
        {
            cout << "Error: " << err.what() << endl;
        }
    }

    cout << "Bye!" << endl;
}

I tried hard to make my range act exactly the same way is in Python (2.7). For instance, if step is 0, an exception is thrown.

I do not have much experience in C++, so any critique is much appreciated.

\$\endgroup\$
  • 1
    \$\begingroup\$ I would add a template function: make_range(). That way you can use template type-deduction provided by functions to automatically get the correctly typed version of range without having the ugly <> at the end. for (auto i : make_range(start, end, step)) \$\endgroup\$ – Martin York Nov 30 '15 at 20:08
  • \$\begingroup\$ Actually, this seems more of a Python 2.7 xrange rather than range. That's not a problem, but worth noting. \$\endgroup\$ – Edward Nov 30 '15 at 20:13
8
\$\begingroup\$

Keeping a Count

You implemented range_iterator to keep a count which you use for equality comparisons. This works, but involves a really awkward formula for getting the count right. Instead, it would be simpler to just round end to be one past the last one, so that range(0, 11, 2) sets start to 0 and end to 12. This would make range_iterator only require two members, and operator!= to just compare the m_value's.

Wrong Type

range_iterator dereferences to int, but should be Int.

Usage

This is awkward:

for (auto i : range<>(start, end, step)) 
//                 ^^^

Rather than having it be up to the user to provide the correct type, you could instead take advantage of template deduction:

template <typename Int>
Range<Int> range(Int start, Int end, Int step) { ... }

You'll have to rename your class template to get this to work. Even better would be to make the function a friend of the class, and make the constructors private.

Keep begin() and end() simple

Once we rewrite range() to be a function template that returns an object, you can move all of the logical checking into it:

template <typename Int>
Range<Int> range(Int start, Int end, Int step) {
    if (step == 0) throw ...;

    if (start <= end) {
       if (step < 0) {
           return {0,0,0};
       }
       return {start, (end + step - 1) / step * step};
    }
    else {
       // etc.
    }
}

This means that begin() just returns range_iterator{m_start, m_step} and end() just returns range_iterator{m_end, m_step}.

Other Overloads

Python also allows for something like range(10), so that's just:

template <class Int>
Range<Int> range(Int end) {
    return {0, end, 1};
}

Iterator Interface

Even though it won't be fully utilized in the simple example of for (auto i : range(10)), you should prefer to provide a complete interface for range_iterator. You're missing operator== and postfix-increment. Prefix-increment (and postfix-increment) should return references to this.

Additionally, you should inherit from std::iterator<std::forward_iterator_tag, int, std::ptrdiff_t, int, int> just in case somebody wants to use range() as a normal container somewhere else.

For example, it'd be nice to support the following:

auto r = range(10);
std::vector<int> v(r.begin(), r.end());

which currently will fail to compile.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Dislike the word should in Additionally, you should inherit from. Though this is the easiest way its not the only way. I would prefer could. But you also need a paragraph describing the requirements for iterators (as currently apart from the name his iterators do not conform to the concept of an iterator) then you can say the best way to implement the requirements is by inheriting from std::iterator<> \$\endgroup\$ – Martin York Dec 1 '15 at 0:48
  • \$\begingroup\$ I don't understand how to get rid of range<> and use only range instead. Any help? \$\endgroup\$ – coderodde Dec 1 '15 at 9:53
  • \$\begingroup\$ @coderodde What part didn't you understand? \$\endgroup\$ – Barry Dec 1 '15 at 14:04
  • \$\begingroup\$ @Barry The second code snippet in the Usage section. \$\endgroup\$ – coderodde Dec 1 '15 at 14:06
  • \$\begingroup\$ @coderodde I made range a function template that returns a Range<Int> - template deduction picks up what Int is, so you don't have to provide it. \$\endgroup\$ – Barry Dec 1 '15 at 14:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.