for_each_cons and for_each_slice, variants of std::for_each working on sliding windows

Question

My goal was to replicate Ruby's each_cons and each_slice:

(0..5).each_cons(3) {|a, b, c| print a, b, c, "\n"}
012
123
234
345

I made two functions, for_each_cons, and for_each_slice. They work similarly to their Ruby counterparts, except the slice is always passed to separate arguments. The slice size is deducted from the passed function's number of arguments.

const std::vector<int> vec = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
for_each_cons(vec, [](int a, int b, int c){
    printf("%d %d %d\n", a, b, c);
});

for_each_slice(vec, [](int a, int b, int c){
    printf("%d %d %d\n", a, b, c);
});

Outputs:

0 1 2
1 2 3
2 3 4
3 4 5

0 1 2
3 4 5

And here's the implementation. For simple code it seems to generate similar assembly to manual implementations, although it's largely because it requires random access iterators.

#include <utility>

namespace detail
{
    template <typename Fun>
    struct function_params
        : public function_params<decltype(&Fun::operator())>
    {};

    template <typename Class, typename Ret, typename... Args> // for lambdas
    struct function_params<Ret(Class::*)(Args...) const> {
        static constexpr int n_args = sizeof...(Args);
    };

    template <typename Class, typename Ret, typename... Args> // for mutable lambdas
    struct function_params<Ret(Class::*)(Args...)> {
        static constexpr int n_args = sizeof...(Args);
    };

    template <typename Ret, typename... Args> // for function pointers
    struct function_params<Ret(*)(Args...)> {
        static constexpr int n_args = sizeof...(Args);
    };

    template<typename It, typename Fun, int N = function_params<Fun>::n_args, std::size_t... I>
    void for_each_cons_impl(It first, It last, Fun f, std::index_sequence<I...>) {
        for(auto it = first; it + N - 1 < last; ++it)
            f(*(it + I)...);
    }

    template<typename It, typename Fun, int N = function_params<Fun>::n_args, typename Indices = std::make_index_sequence<N>>
    void for_each_cons(It first, It last, Fun f) {
        for_each_cons_impl(first, last, f, Indices());    
    }

    template<typename It, typename Fun, int N = function_params<Fun>::n_args, std::size_t... I>
    void for_each_slice_impl(It first, It last, Fun f, std::index_sequence<I...>) {
        for(auto it = first; it + N - 1 < last; it += N)
            f(*(it + I)...);
    }

    template<typename It, typename Fun, int N = function_params<Fun>::n_args, typename Indices = std::make_index_sequence<N>>
    void for_each_slice(It first, It last, Fun f) {
        for_each_slice_impl(first, last, f, Indices());    
    }
}

template<typename It, typename Fun>
void for_each_cons(It first, It last, Fun f) {
    detail::for_each_cons(first, last, f);
}

template<typename Rng, typename Fun>
void for_each_cons(Rng &&rng, Fun f) {
    for_each_cons(rng.begin(), rng.end(), f);
}

template<typename It, typename Fun>
void for_each_slice(It first, It last, Fun f) {
    detail::for_each_slice(first, last, f);
}

template<typename Rng, typename Fun>
void for_each_slice(Rng &&rng, Fun f) {
    detail::for_each_slice(rng.begin(), rng.end(), f);
}

Any hints and criticism are welcome.

Answer 1

The two main issues I see are that

1. You require RandomAccessIterators although these should be implementable with ForwardIterators as well since you never have to skip an element.
2. This will fail for functors with an overloaded or templated operator() such as those with auto parameters

Also, what happens if the range's length isn't divisible by the step size in the second case?

As 2 isn't really possible to solve since in the overloaded case there may be multiple valid values for n_args you could allow the user to pass the number of arguments as a first parameter. You could then default that value to 0 (which is never valid) and infer the number of parameters as you currently do if it is set to 0.

Answer 2

There are at least a couple of things that you could improve in your code without changing the way it works:

• I would have used std::size_t for the number of parameters of a function. It is more consistent with std::index_sequence that uses std::size_t underneath.

• You don't have to always pollute the template parameters list when using the indices trick. If you don't need a type alias or a constant in the parameters type or in the return type, then simply move it to the body of the function. It will make it easier for users to understand the signature:

  template<typename It, typename Fun, std::size_t... I>
  void for_each_cons_impl(It first, It last, Fun f, std::index_sequence<I...>) {
      static constexpr std::size_t N = function_params<Fun>::n_args;
      for(auto it = first; it + N - 1 < last; ++it)
          f(*(it + I)...);
  }
  
  template<typename It, typename Fun>
  void for_each_cons(It first, It last, Fun f) {
      static constexpr std::size_t N = function_params<Fun>::n_args;
      using Indices = std::make_index_sequence<N>;
      for_each_cons_impl(first, last, f, Indices());    
  }
  

• Functions are sometimes big objects, you could use forwarding references and std::forward instead of passing everything by value:

  template<typename It, typename Fun>
  void for_each_cons(It first, It last, Fun&& f) {
      static constexpr std::size_t N = function_params<Fun>::n_args;
      using Indices = std::make_index_sequence<N>;
      for_each_cons_impl(first, last, std::forward<Fun>(f), Indices());    
  }
  

  However, you will have to use function_params<std::decay_t<Fun>>::n_args instead of function_params<Fun>::n_args in order for this to run.

• But actually, your implementation of function_params lacks many cases for volatile functions as well as functions with ref-qualifiers. You could avoid the std::decay_t of the previous point if you completed your specialization set. However I have to admit that there is no simple workaround and that you have to write every specialization by hand. That's cumbersome, but unless you want to dirty your hands with generative macros, that's currently the only way.

• Since you are using random-access iterators anyway, you might as well use the brackets syntax instead of the addition & dereference syntax:

  template<typename It, typename Fun, std::size_t... I>
  void for_each_cons_impl(It first, It last, Fun&& f, std::index_sequence<I...>) {
      static constexpr std::size_t N = function_params<Fun>::n_args;
      for(auto it = first; it + N - 1 < last; ++it)
          std::forward<Fun>(f)(it[I]...);
  }
  

• On the other hand, you can also make your code work for forward iterators with only a few changes, replacing the pointer arithmetic operations by calls to std::distance, std::advance and std::next:

  template<typename It, typename Fun, int N = function_params<Fun>::n_args, std::size_t... I>
  void for_each_slice_impl(It first, It last, Fun f, std::index_sequence<I...>) {
      for(auto it = first; std::distance(it, last) > N - 1; std::advance(it, N))
          f(*std::next(it, I)...);
  }
  

  Keep in mind that the complexity is higher for forward iterators than for random-access iterators though. There might be ways to reduce it by manually keeping track of the distance between it and last instead of computing it by advancing the iterators for every iteration of the loop.

• Since we are dealing with iterators, I would have kept names consistent with those in the standard library and named them begin and end instead of first and last. last is especially confusing since the last iterator is conceptually the one before the end iterator, it was even proposed to add the function last to containers in N3585 to return and iterator pointing to the last element.