5
\$\begingroup\$

Python has thatrange:

>>> 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 demomain.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 myrange act exactly the same way is in Python (2.7). For instance, ifstep is 0, an exception is thrown.

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

Barry's user avatar
Barry
18.6k1 gold badge41 silver badges92 bronze badges
askedNov 30, 2015 at 18:19
coderodde's user avatar
\$\endgroup\$
2
  • 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 ofrange without having the ugly<> at the end.for (auto i : make_range(start, end, step))\$\endgroup\$CommentedNov 30, 2015 at 20:08
  • \$\begingroup\$Actually, this seems more of a Python 2.7xrange rather thanrange. That's not a problem, but worth noting.\$\endgroup\$CommentedNov 30, 2015 at 20:13

1 Answer1

8
\$\begingroup\$

Keeping a Count

You implementedrange_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 roundend to be one past the last one, so thatrange(0, 11, 2) setsstart to0 andend to12. This would makerange_iterator only require two members, andoperator!= to just compare them_value's.

Wrong Type

range_iterator dereferences toint, but should beInt.

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 afriend of the class, and make the constructors private.

Keepbegin() andend() simple

Once we rewriterange() 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 thatbegin() just returnsrange_iterator{m_start, m_step} andend() just returnsrange_iterator{m_end, m_step}.

Other Overloads

Python also allows for something likerange(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 offor (auto i : range(10)), you should prefer to provide a complete interface forrange_iterator. You're missingoperator== and postfix-increment. Prefix-increment (and postfix-increment) should return references to this.

Additionally, you should inherit fromstd::iterator<std::forward_iterator_tag, int, std::ptrdiff_t, int, int> just in case somebody wants to userange() 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.

answeredNov 30, 2015 at 18:38
Barry's user avatar
\$\endgroup\$
5
  • \$\begingroup\$Dislike the wordshould inAdditionally, you should inherit from. Though this is the easiest way its not the only way. I would prefercould. But you also need a paragraph describing the requirements for iterators (as currently apart from the name his iterators do not conform to theconcept of an iterator) then you can say the best way to implement the requirements is by inheriting fromstd::iterator<>\$\endgroup\$CommentedDec 1, 2015 at 0:48
  • \$\begingroup\$I don't understand how to get rid ofrange<> and use onlyrange instead. Any help?\$\endgroup\$CommentedDec 1, 2015 at 9:53
  • \$\begingroup\$@coderodde What part didn't you understand?\$\endgroup\$CommentedDec 1, 2015 at 14:04
  • \$\begingroup\$@Barry The second code snippet in theUsage section.\$\endgroup\$CommentedDec 1, 2015 at 14:06
  • \$\begingroup\$@coderodde I maderange a function template that returns aRange<Int> - template deduction picks up whatInt is, so you don't have to provide it.\$\endgroup\$CommentedDec 1, 2015 at 14:35

You mustlog in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.