4
\$\begingroup\$

As my answer to Iterating over an odd (even) elements only in a range-based loop, I wrote this function, with the following driver program and output:

#include <array>
#include <vector>
#include <iterator>

// Forward iteration from begin to end by step size N.
template<typename Container, typename Function>
void for_each_by_n( Container&& cont, Function f, unsigned increment_by = 1) {
    if ( increment_by == 0 ) return; // must check this for no op

    using std::begin;
    auto it = begin(cont);

    using std::end;
    auto end_it = end(cont);

    while( it != end_it ) {
        f(*it);
        for ( unsigned n = 0; n < increment_by; ++n ) {
            if ( it == end_it ) return;
            ++it;
        }
    }
}

int main() {
    std::array<int,8> arr{ 0,1,2,3,4,5,6,7 };
    std::vector<double> vec{ 1.2, 1.5, 1.9, 2.5, 3.3, 3.7, 4.2, 4.8 };

    auto l = [](auto& v) { std::cout << v << ' '; };

    for_each_by_n(arr, l); std::cout << '\n';
    for_each_by_n(vec, l); std::cout << '\n';

    for_each_by_n(arr, l, 2); std::cout << '\n';
    for_each_by_n(arr, l, 4); std::cout << '\n';

    for_each_by_n(vec, l, 3); std::cout << '\n';
    for_each_by_n(vec, l, 5); std::cout << '\n';

    for_each_by_n(arr, l, 8); std::cout << '\n';
    for_each_by_n(vec, l, 8); std::cout << '\n';

    // sanity check to see if it doesn't go past end.
    for_each_by_n(arr, l, 9); std::cout << '\n';
    for_each_by_n(vec, l, 9); std::cout << '\n';

    return 0;
}

Output

 0 1 2 3 4 5 6 7
 1.2 1.5 1.9 2.5 3.3 3.7 4.2 4.8
 0 2 4 6 
 0 4
 1.2 2.5 4.2
 1.2 3.7
 0
 1.2
 0
 1.2

I did my best to check for possible bugs, corner cases etc.

What I would like to know about my function above:

  • Does this follow modern C++ standards?
  • Is there any room for improvements?
  • Did I miss any possible bugs that I might have overlooked?
  • Would this be considered readable, reliable, generic, portable, cross-platform and reusable?
  • Do I have to worry about any const correctness, type deduction, cache misses and the like?
  • -Note-: I know the above function is not in a designated namespace; that is not of concern here. I can do that without hassle or trouble.

Let me know what you think; I'm looking forward to any and all feedback.

I would like to know ahead of time for I am thinking about adding a second unsigned integer parameter to this function. It would allow the user to choose the index location they want to use for their starting position. This parameter would default to 0.

\$\endgroup\$
5
\$\begingroup\$
  • We could use Container::size_type or std::size_t for the increment_by parameter, or (as standard algorithms like std::for_each_n seem to do) make it a template argument and use the same type while incrementing n.

  • The standard library algorithms take iterators rather than containers. You mention allowing users to specify an "index location they want to use for their starting position", which would be accomplished by passing an iterator range instead of calling begin and end.

  • I don't think the default increment_by argument value is useful. If we needed a step size of 1, we'd call std::for_each or use a range-based for loop.

  • std::for_each returns the function object (which can be helpful for something like summing values). We could do the same.

  • Follow the standard library conventions with naming template arguments (e.g. name the minimum required iterator type, make it clear that the function is a unary function).


Modified version:

template<class InputIt, class Size, class UnaryFunction>
UnaryFunction for_each_by_n(InputIt begin, InputIt end, Size step, UnaryFunction f) {

    if (step == 0)
        return f;

    while (begin != end)
    {
        f(*begin);

        for (Size n = 0; n != step; ++n)
        {
            if (begin == end)
                return f;

            ++begin;
        }
    }

    return f;
}

(edit: removed unnecessary std::move per Juho's comment).

\$\endgroup\$
  • 1
    \$\begingroup\$ It seems like it's a better idea to just return by-copy instead of move; i.e., you gain by nothing by doing it. \$\endgroup\$ – Juho Feb 25 at 18:18
  • \$\begingroup\$ @Juho I did vaguely wonder about that. The std::for_each reference that I was looking at explicitly mentions std::move(f) for the return value, which is kinda weird: en.cppreference.com/w/cpp/algorithm/for_each \$\endgroup\$ – user673679 Feb 25 at 18:29
  • 1
    \$\begingroup\$ I see, interesting. Then again the "possible implementation" again does not use the move. \$\endgroup\$ – Juho Feb 25 at 18:32
  • 1
    \$\begingroup\$ Yep. Looks like it's just an odd quirk: stackoverflow.com/questions/43163801/… \$\endgroup\$ – user673679 Feb 25 at 18:34
  • \$\begingroup\$ The fact that f is a parameter of the function, rather than a local variable, adds extra complication here. I'm no longer sure what's correct (reading that answer is certainly useful; making the parameter accept a forwarding reference may make a difference, too. \$\endgroup\$ – Toby Speight Feb 25 at 18:47
3
\$\begingroup\$

A minor tweak might be to test for end_it within the condition expression of the for:

    for (unsigned n = 0;  n < increment_by && it != end_it;  ++n) {
        ++it;
    }

Incorporated into user673679's version, that becomes (untested):

template<class InputIt, class Size, class UnaryFunction>
UnaryFunction for_each_by_n(InputIt begin, InputIt end, Size step,
                            UnaryFunction&& f)
{
    if (step > 0) {    
        while (begin != end) {
            f(*begin);

            for (Size n = 0u;  n != step && begin != end;  ++n) {
                ++begin;
            }
        }
    }

    return f;
}
\$\endgroup\$
2
\$\begingroup\$

Well, there is one thing you should consider, as you aim for genericity and composability:
Doing things the other way around.

Rather than writing a for_each_by_n() (which would be better named for_each_stride()), use std::for_each() respectively the for-range-loop, and appropriate views to adapt the range.

As an example, with range-v3, you would use the view:

auto view = view::stride(container, n);
\$\endgroup\$

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.