2
\$\begingroup\$

Problem my code solves:

Lack of lightweight numeric range classes which can tell if arbitrary values lie inside them, supporting more than just simple exclusive max modes and integers, and that support iteration with configurable stepsizes.

What I think My code does:

  • Provides a lightwieght class encapsulating a numeric range
  • Iteration Should be valid for any <= 64bit numeric type, at the very least
    • std::uint8_t, std::uint16_t, std::uint32_t, std::uint64_t
    • std::int8_t, std::int16_t, std::int32_t, std::int64_t
    • half, float, double.
  • Should be valid with out iteration otherwise.
  • Guaranteed iteration starts and ends
  • Proper exclusive, inclusive, exclusive max, exclusive min check for values (shown in top quarter of code).
  • Iterators that start and end at exclusive, inclusive, exclusive max and exclusive min (bottom part of code) with configurable step size.
  • Reverse iteration with configurable step size.

Concerns with my code:

  • I'm not really sure how I handled the iterators was "correct" even though it works, I'm not exactly sure how I'm supposed to handle iterators that aren't reference/pointer types underneath.
  • Are there any important facilities I've missed that should be in such a class (not necisarily how to implement them)
  • Are there any C++ features I'm not using correctly/idiomatically or are missing?
  • Architectural problems?
  • Problematic API choices?
  • Code clarity from both user perspective and developer perspective.
  • Any other issues

Here is the code

(godbolt link as well)

numeric_range.h

#pragma once
#include <cassert>
#include <type_traits>
#include <iterator>

namespace stx {

    template<typename T>
    class numeric_range {
    public:
        numeric_range(const T &min, const T &max) : m_min(min), m_max(max) {
            assert(m_min <= m_max);
        }

        numeric_range(const T &max) : m_min{}, m_max(max) {
            assert(m_min <= m_max);
        }

        T min() const {
            return m_min;
        }

        T max() const {
            return m_max;
        }

        T size() const {
            return m_max - m_min;
        }

        numeric_range &set_max(const T &value) {
            assert(value >= m_min);
            m_max = value;
            return *this;
        }

        numeric_range &set_min(const T &value) {
            assert(value <= m_max);
            m_min = value;
            return *this;
        }

        //inclusive contains min and max
        bool contains_inclusive(const T& value) const{
            return (m_min <= value) && (value <= m_max);
         }

        //exclusive excludes both min and max 
        bool contains_exclusive(const T& value) const{
            return (m_min < value) && (value < m_max);
        }

        //exclude max includes min but excludes max
        bool contains_exclude_max(const T& value) const{
            return (m_min <= value) && (value < m_max);
        }

        //exclude min includes max but excludes min
        bool contains_exclude_min(const T& value) const{
            return (m_min < value) && (value <= m_max);
        }

        //excludes max
        bool contains(const T &value) const {
            return contains_exclude_max(value);
        }

        class iterator {
        public:
            iterator(std::size_t step_index, std::size_t step_max,
                     const T &min, const T &max) :
                    m_step_index(step_index), m_step_max(step_max),
                    m_min(min), m_max(max) {
                assert(step_index <= step_max+1);
                if constexpr (std::is_integral_v<T>) {
                    //we need to make sure if we are doing int stuff
                    //we can't have partial steps, logic gets hairy.
                    assert((m_max - m_min) % step_max == 0);
                }
            }

            iterator &operator++() {
                m_step_index += 1;
                return *this;
            }

            iterator operator++(int) {
                iterator retval = *this;
                ++(*this);
                return retval;
            }

            bool operator==(iterator other) const {
                return m_step_index == other.m_step_index
                       && m_step_max == other.m_step_max
                       && m_min == other.m_min
                       && m_max == other.m_max;
            }

            bool operator!=(iterator other) const {
                return !(*this == other);
            }

            T operator*() const{
                //for integers, works perfectly. for floats, you can't get perfect but
                //gaurantees when step index is 0
                //perfect stepping for integers
                if constexpr (std::is_integral_v<T>) {
                    return m_min +
                           ((m_max - m_min) / m_step_max) * m_step_index;
                } else {
                    // floating point needs to be handled differently to
                    // guarantee that starts and ends are 0 and 1.
                    // no worry of error from range addition
                    return ((m_step_max - m_step_index) /
                            static_cast<T>(m_step_max)) * m_min +
                           (m_step_index / static_cast<T>(m_step_max)) * m_max;
                }
            }
            // iterator traits
            using difference_type = T;
            using value_type = T;
            using pointer = std::size_t;
            using reference = T &;
            using iterator_category = std::forward_iterator_tag;
        private:
            std::size_t m_step_index;
            std::size_t m_step_max;
            T m_min;
            T m_max;
        };

        class reverse_iterator {
        public:
            reverse_iterator(std::size_t step_index, std::size_t step_max,
                     const T &min, const T &max) :
                    m_step_index(step_index), m_step_max(step_max),
                    m_min(min), m_max(max) {
                assert(step_index <= step_max+1);
                if constexpr (std::is_integral_v<T>) {
                    //we need to make sure if we are doing int stuff
                    //we can't have partial steps, logic gets hairy.
                    assert((m_max - m_min) % step_max == 0);
                }
            }

            reverse_iterator &operator++() {
                m_step_index += 1;
                return *this;
            }

            reverse_iterator operator++(int) {
                reverse_iterator retval = *this;
                ++(*this);
                return retval;
            }

            bool operator==(reverse_iterator other) const {
                return m_step_index == other.m_step_index
                       && m_step_max == other.m_step_max
                       && m_min == other.m_min
                       && m_max == other.m_max;
            }

            bool operator!=(reverse_iterator other) const {
                return !(*this == other);
            }

            T operator*() const{
                //for integers, works perfectly. for floats, you can't get perfect but
                //gaurantees when step index is 0
                //perfect stepping for integers
                if constexpr (std::is_integral_v<T>) {
                    //negation shouldn't
                    return m_max -
                           ((m_max - m_min) / m_step_max) * (m_step_index);
                } else {
                    // floating point needs to be handled differently to
                    // guarantee that starts and ends are 0 and 1.
                    // no worry of error from range addition
                    return ((m_step_max - m_step_index) /
                            static_cast<T>(m_step_max)) * m_max +
                           (m_step_index / static_cast<T>(m_step_max)) * m_min;
                }
            }
            // iterator traits
            using difference_type = T;
            using value_type = T;
            using pointer = std::size_t;
            using reference = T &;
            using iterator_category = std::forward_iterator_tag;
        private:
            std::size_t m_step_index;
            std::size_t m_step_max;
            T m_min;
            T m_max;
        };

        template<class TIterator>
        class exclude_end_iterator_range {
        public:
            exclude_end_iterator_range(std::size_t step_max, const T &min, const T &max)
                    : m_step_max(step_max), m_min(min), m_max(max) {

            }
            TIterator begin()const{
                return TIterator(0, m_step_max, m_min, m_max);
            }
            TIterator end()const{
                return TIterator(m_step_max, m_step_max, m_min, m_max);
            }

        private:
            std::size_t m_step_max;
            T m_min;
            T m_max;
        };

        template<class TIterator>
        class exclude_begin_iterator_range {
        public:
            exclude_begin_iterator_range(std::size_t step_max, const T &min, const T &max)
            : m_step_max(step_max), m_min(min), m_max(max) {

            }
            TIterator begin()const{
                return TIterator(1, m_step_max, m_min, m_max);
            }
            TIterator end()const{
                return TIterator(m_step_max+1, m_step_max, m_min, m_max);
            }
        private:
            std::size_t m_step_max;
            T m_min;
            T m_max;
        };

        template<class TIterator>
        class inclusive_iterator_range {
        public:
            inclusive_iterator_range(std::size_t step_max, const T &min, const T &max)
            : m_step_max(step_max), m_min(min), m_max(max) {

            }
            TIterator begin() const{
                return TIterator(0, m_step_max, m_min, m_max);
            }
            TIterator end() const{
                return TIterator(m_step_max+1, m_step_max, m_min, m_max);
            }
        private:
            std::size_t m_step_max;
            T m_min;
            T m_max;
        };

        template<class TIterator>
        class exclusive_iterator_range {
        public:
            exclusive_iterator_range(std::size_t step_max, const T &min, const T &max)
            : m_step_max(step_max), m_min(min), m_max(max) {

            }
            TIterator begin()const{
                return TIterator(1, m_step_max, m_min, m_max);
            }
            TIterator end()const{
                return TIterator(m_step_max, m_step_max, m_min, m_max);
            }
        private:
            std::size_t m_step_max;
            T m_min;
            T m_max;
        };

        exclude_end_iterator_range<iterator> forward_exclude_max(const T& step_size){
            std::size_t step_max = size()/step_size;
            return exclude_end_iterator_range<iterator>(step_max, m_min, m_max);
        }

        exclude_begin_iterator_range<iterator> forward_exclude_min(const T& step_size){
            std::size_t step_max = size()/step_size;
            return exclude_begin_iterator_range<iterator>(step_max, m_min, m_max);
        }

        exclusive_iterator_range<iterator> forward_exclusive(const T& step_size){
            std::size_t step_max = size()/step_size;
            return exclusive_iterator_range<iterator>(step_max, m_min, m_max);
        }

        inclusive_iterator_range<iterator> forward_inclusive(const T& step_size){
            std::size_t step_max = size()/step_size;
            return inclusive_iterator_range<iterator>(step_max, m_min, m_max);
        }

        //swap internals because reverse iterator causes min and max to swap
        // from being begin and end respectively to end and begin
        exclude_begin_iterator_range<reverse_iterator> reverse_exclude_max(const T& step_size){
            std::size_t step_max = size()/step_size;
            return exclude_begin_iterator_range<reverse_iterator>(step_max, m_min, m_max);
        }

        exclude_end_iterator_range<reverse_iterator> reverse_exclude_min(const T& step_size){
            std::size_t step_max = size()/step_size;
            return exclude_end_iterator_range<reverse_iterator>(step_max, m_min, m_max);
        }

        exclusive_iterator_range<reverse_iterator> reverse_exclusive(const T& step_size){
            std::size_t step_max = size()/step_size;
            return exclusive_iterator_range<reverse_iterator>(step_max, m_min, m_max);
        }

        inclusive_iterator_range<reverse_iterator> reverse_inclusive(const T& step_size){
            std::size_t step_max = size()/step_size;
            return inclusive_iterator_range<reverse_iterator>(step_max, m_min, m_max);
        }


        //returns a forward iterator that excludes the end of the range (max)
        iterator begin()const{
            exclude_end_iterator_range<iterator> iterator_range(static_cast<std::size_t>(size()), m_min, m_max);
            return iterator_range.begin();
        }

        //returns a forward iterator that excludes the end of the range (max)
        iterator end()const{
            exclude_end_iterator_range<iterator> iterator_range(static_cast<std::size_t>(size()), m_min, m_max);
            return iterator_range.end();
        }

        //returns a reverse iterator that excludes the end of the range (min)
        reverse_iterator rbegin()const{
            exclude_end_iterator_range<reverse_iterator> iterator_range(static_cast<std::size_t>(size()), m_min, m_max);
            return iterator_range.begin();
        }

        //returns a reverse iterator that excludes the end of the range (min)
        reverse_iterator rend()const{
            exclude_end_iterator_range<reverse_iterator> iterator_range(static_cast<std::size_t>(size()), m_min, m_max);
            return iterator_range.end();
        }

    private:
        T m_min;
        T m_max;
    };

    template<typename T>
    bool
    operator==(const numeric_range<T> &lhs, const numeric_range<T> &rhs) {
        return lhs.min() == rhs.min() && lhs.max() == rhs.max();
    }

    template<typename T>
    bool
    operator!=(const numeric_range<T> &lhs, const numeric_range<T> &rhs) {
        return !(lhs == rhs);
    }
}

main.cpp

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

int main(){
    stx::numeric_range<float> frange(0.0, 10.0);
    stx::numeric_range<int> irange(0, 10);

    std::cout << "frange contains 5.435: " <<  frange.contains(5.435) << "\n";
    std::cout << "frange does not contain exclusive 0.0: " <<  !frange.contains_exclusive(0.0) << "\n";
    std::cout << "irange contains 5: " <<  frange.contains(5) << "\n";
    std::cout << "irange does not contain exclusive 10: " <<  !frange.contains_exclusive(10) << "\n";

    std::cout << "frange iterate: ";
    for(auto i : frange){
        std::cout << i << ",";
    }
    std::cout << "\n";
    std::cout << "irange iterate: ";
    for(auto i : irange){
        std::cout << i << ",";
    }
    std::cout << "\n";

    std::cout << "frange 0.5 iterate: ";
    for(auto i : frange.forward_exclude_max(0.5)){
        std::cout << i << ",";
    }
    std::cout << "\n";
    std::cout << "irange 2 iterate: ";
    for(auto i : irange.forward_exclude_max(2)){
        std::cout << i << ",";
    }
    std::cout << "\n";

    return 0;
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$
  1. Arguments of scalar types are better passed by value.

  2. It makes sense for iterator types to befriend the range class itself and hide their constructors private.

  3. As a rule, it is undefined to compare iterators into different containers. Consequently, you can reduce your iterators comparison to but a single relational iterator (add a debug-time diagnostic for checking range ends if you like).

  4. Next, iterators don't need to keep track of range ends. The range itself knows its max when constructing an end iterator, the iterator itself needs not to know it's past the end. Finally, you don't need to execute division on each increment, even if it's optimized out (I wonder if every 17-compliant compiler is capable of that).

  5. Iterators are trivially random-access, so you can add more relational and arithmetical operators to them.

  6. Perhaps it makes sense to add a setter that mutates both ends of a range in a single call. In addition, operators like union of adjacent ranges might prove useful.

  7. gaurantees :)

\$\endgroup\$
4
  • \$\begingroup\$ so for 3 you are saying that you shouldn't be comparing iterators from different objects anyway because no one should expect that to work? For 4, don't I need to know the size still? for integers I could use a step size with out knowing the end, but for floats I can't guarantee addition will add up to the end. For floats I can't pre-multiply the division because I won't be able to guarantee that things like ((m_max/m_step_max) * m_step_index) = m_max. \$\endgroup\$
    – Krupip
    Mar 15, 2020 at 21:51
  • 1
    \$\begingroup\$ 3. Yes. 4. Ok, they make sense, but only for FP, for better precision. \$\endgroup\$
    – bipll
    Mar 16, 2020 at 7:08
  • \$\begingroup\$ do you think it would be worth it to just create two enable-ifed classes for the iterator types then since there is already type specific logic in each type (int and float) to take advantage of this? \$\endgroup\$
    – Krupip
    Mar 16, 2020 at 14:40
  • 1
    \$\begingroup\$ Yes, should work. \$\endgroup\$
    – bipll
    Mar 17, 2020 at 6:50

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.