Skip to main content
added 1 character in body
Source Link
G. Sliepen
  • 61.7k
  • 3
  • 61
  • 152

Unnecessary restriction of the value_type

You restrict the value_type of a range to the integral types, but I see no reason why that is necessary. It would work equally well with float and double, and any other type that you can add and compare with (consider for example std::chrono::duration).

This can be taken further to have different types for start, stop and step. For example, instead of integers, consider using an iterator for start and stop, and being able to do:

std::vector<std::string> names{...};

// Print every other name
for (auto &name: range(names.begin(), names.end(), 2))
   std::cout << name*name << '\n';

This can be done like so:

template <typename StopType = int, typename StartType = StopType,
          typename StepType = decltype(StopType{} - StartType{})>
struct range: std::ranges::view_interface<range<StopType, StartType, StepType>> {
    range(): range(0) {}
    explicit range(StopType stop): range(StopType{}, stop) {}
    explicit range(StartType start, StopType stop,
                   StepType step = decltype(stop - start){1}):
        m_value{start}, m_stop{stop}, m_step{step}
    {
        if (step == StepType{})
            throw std::domain_error("Step cannot be zero");
    }
    ...
};

Then even the following would work:

std::vector<std::string> words{"Aap", "Noot", "Mies", "Win""Wim", "Zus", "Jet"};
for (auto word: range(words.begin(), words.end(), 2))
    std::cout << *word << "\n";

for (auto t: range(std::chrono::minutes(10)))
    std::cout << std::chrono::duration_cast<std::chrono::seconds>(t).count() << '\n';

Make begin() and end() member functions

The functions begin() and end() should be member functions, not friend functions. If they are member functions, std::begin(range(...)) will still work, but if it is a friend, then range(...).begin() will fail to compile.

Unnecessary restriction of the value_type

You restrict the value_type of a range to the integral types, but I see no reason why that is necessary. It would work equally well with float and double, and any other type that you can add and compare with (consider for example std::chrono::duration).

This can be taken further to have different types for start, stop and step. For example, instead of integers, consider using an iterator for start and stop, and being able to do:

std::vector<std::string> names{...};

// Print every other name
for (auto &name: range(names.begin(), names.end(), 2))
   std::cout << name << '\n';

This can be done like so:

template <typename StopType = int, typename StartType = StopType,
          typename StepType = decltype(StopType{} - StartType{})>
struct range: std::ranges::view_interface<range<StopType, StartType, StepType>> {
    range(): range(0) {}
    explicit range(StopType stop): range(StopType{}, stop) {}
    explicit range(StartType start, StopType stop,
                   StepType step = decltype(stop - start){1}):
        m_value{start}, m_stop{stop}, m_step{step}
    {
        if (step == StepType{})
            throw std::domain_error("Step cannot be zero");
    }
    ...
};

Then even the following would work:

std::vector<std::string> words{"Aap", "Noot", "Mies", "Win", "Zus", "Jet"};
for (auto word: range(words.begin(), words.end(), 2))
    std::cout << *word << "\n";

for (auto t: range(std::chrono::minutes(10)))
    std::cout << std::chrono::duration_cast<std::chrono::seconds>(t).count() << '\n';

Make begin() and end() member functions

The functions begin() and end() should be member functions, not friend functions. If they are member functions, std::begin(range(...)) will still work, but if it is a friend, then range(...).begin() will fail to compile.

Unnecessary restriction of the value_type

You restrict the value_type of a range to the integral types, but I see no reason why that is necessary. It would work equally well with float and double, and any other type that you can add and compare with (consider for example std::chrono::duration).

This can be taken further to have different types for start, stop and step. For example, instead of integers, consider using an iterator for start and stop, and being able to do:

std::vector<std::string> names{...};

// Print every other name
for (auto &name: range(names.begin(), names.end(), 2))
   std::cout << *name << '\n';

This can be done like so:

template <typename StopType = int, typename StartType = StopType,
          typename StepType = decltype(StopType{} - StartType{})>
struct range: std::ranges::view_interface<range<StopType, StartType, StepType>> {
    range(): range(0) {}
    explicit range(StopType stop): range(StopType{}, stop) {}
    explicit range(StartType start, StopType stop,
                   StepType step = decltype(stop - start){1}):
        m_value{start}, m_stop{stop}, m_step{step}
    {
        if (step == StepType{})
            throw std::domain_error("Step cannot be zero");
    }
    ...
};

Then even the following would work:

std::vector<std::string> words{"Aap", "Noot", "Mies", "Wim", "Zus", "Jet"};
for (auto word: range(words.begin(), words.end(), 2))
    std::cout << *word << "\n";

for (auto t: range(std::chrono::minutes(10)))
    std::cout << std::chrono::duration_cast<std::chrono::seconds>(t).count() << '\n';

Make begin() and end() member functions

The functions begin() and end() should be member functions, not friend functions. If they are member functions, std::begin(range(...)) will still work, but if it is a friend, then range(...).begin() will fail to compile.

Source Link
G. Sliepen
  • 61.7k
  • 3
  • 61
  • 152

Unnecessary restriction of the value_type

You restrict the value_type of a range to the integral types, but I see no reason why that is necessary. It would work equally well with float and double, and any other type that you can add and compare with (consider for example std::chrono::duration).

This can be taken further to have different types for start, stop and step. For example, instead of integers, consider using an iterator for start and stop, and being able to do:

std::vector<std::string> names{...};

// Print every other name
for (auto &name: range(names.begin(), names.end(), 2))
   std::cout << name << '\n';

This can be done like so:

template <typename StopType = int, typename StartType = StopType,
          typename StepType = decltype(StopType{} - StartType{})>
struct range: std::ranges::view_interface<range<StopType, StartType, StepType>> {
    range(): range(0) {}
    explicit range(StopType stop): range(StopType{}, stop) {}
    explicit range(StartType start, StopType stop,
                   StepType step = decltype(stop - start){1}):
        m_value{start}, m_stop{stop}, m_step{step}
    {
        if (step == StepType{})
            throw std::domain_error("Step cannot be zero");
    }
    ...
};

Then even the following would work:

std::vector<std::string> words{"Aap", "Noot", "Mies", "Win", "Zus", "Jet"};
for (auto word: range(words.begin(), words.end(), 2))
    std::cout << *word << "\n";

for (auto t: range(std::chrono::minutes(10)))
    std::cout << std::chrono::duration_cast<std::chrono::seconds>(t).count() << '\n';

Make begin() and end() member functions

The functions begin() and end() should be member functions, not friend functions. If they are member functions, std::begin(range(...)) will still work, but if it is a friend, then range(...).begin() will fail to compile.