Return to Answer

I am not focussing on the algorithm, but more on C++ idioms.

Very good statement of what you want out of this. Great!

I suggest you familiarize yourself with the Core Guidelines being cataloged by Bjarne Stroustrup and Herb Sutter.

#define UPPER_BOUND 1000

Don't use #define for constants or "functions" (⧺ES.31).

This is one of the first things ancient C++ brought to the table! Instead of #define with all its problems, we got const and inline.

The loop itself is sound enough, even using the prefix increment. You would need additional (non-std) libraries to write that neater using a range-for.

But, you should not write the loop at all! Stroustrup's videos will say: "Don't 'write code' — use algorithms." You want count_if, because you are asking "how many items satisfy this predicate". (update: On closer reading I see you are not counting but totaling. This is exactly the hazard of extracting the high level meaning from a low-level loop!) But again, you just have counting numbers not a collection to go through, which is not the normal case. std::generate_n will produce the list of numbers, but not in a form that can be fed to the algorithms! It pushes, while count_if pulls.

Writing the for loop, even the legacy form, is just so easy that it is hard to argue for doing it differently.

But with additional libraries: Boost.Range v2, or the upcoming Range.v3, it will be simple. Many programmers use Boost or have their own counting range (see my own tutorial and the cartoon that goes with it ☺).

So, with library in hand, I might write something like:

auto r = int_range (1, upper_bound);
auto wanted = [](int i){return (i % 3 == 0 || i % 5 == 0); }
auto sum = accumulate (r|filtered(wanted));

Since the actual work here, sum += i is so trivial that any elaboration on it makes it more code. But imagine a more realistic complex situation in business logic. The algorithm, accumulate, is named for what you are doing. The selection criteria is isolated in its own statement and named wanted, so it is easier to edit the logic later without breaking anything.

The two issues here are that the desired idioms are leading ahead of proper library/compiler support; and with a trivial program it seems artificial and convoluted to put any formal structure or “high engineering” around it.

What might be useful is to verify that your sum does not overflow. That’s something that I eyeballed when reviewing, so a comment to the effect that you considered that is a good idea.

// sum will be a subset of the sum of all numbers in the range,
// so must be < (n+1)*(n/2) ≈ 500'000, which is well within the range
// of a 32-bit int.

I am not focussing on the algorithm, but more on C++ idioms.

Very good statement of what you want out of this. Great!

I suggest you familiarize yourself with the Core Guidelines being cataloged by Bjarne Stroustrup and Herb Sutter.

#define UPPER_BOUND 1000

Don't use #define for constants or "functions" (⧺ES.31).

This is one of the first things ancient C++ brought to the table! Instead of #define with all its problems, we got const and inline.

The loop itself is sound enough, even using the prefix increment. You would need additional (non-std) libraries to write that neater using a range-for.

But, you should not write the loop at all! Stroustrup’s videos will say: “Don’t ‘write code’ — use algorithms.” You want count_if, because you are asking “how many items satisfy this predicate”.   (update: On closer reading I see you are not counting but totaling. This is exactly the hazard of extracting the high level meaning from a low-level loop!) But again, you just have counting numbers not a collection to go through, which is not the normal case.   std::generate_n will produce the list of numbers, but not in a form that can be fed to the algorithms! It pushes, while count_if pulls.

Writing the for loop, even the legacy form, is just so easy that it is hard to argue for doing it differently.

But with additional libraries:   Boost.Range v2, or the upcoming Range.v3, it will be simple. Many programmers use Boost or have their own counting range (see my own tutorial and the cartoon that goes with it ☺).

So, with library in hand, I might write something like:

auto r = int_range (1, upper_bound);
auto wanted = [](int i){return (i % 3 == 0 || i % 5 == 0); }
auto sum= accumulate (r|filtered(wanted));

Since the actual work here, sum+=i is so trivial that any elaboration on it makes it more code. But imagine a more realistic complex situation in business logic. The algorithm, accumulate, is named for what you are doing. The selection criteria is isolated in its own statement and named wanted, so it is easier to edit the logic later without breaking anything.

The two issues here are that the desired idioms are leading ahead of proper library/compiler support; and with a trivial program it seems artificial and convoluted to put any formal structure or “high engineering” around it.

What might be useful is to verify that your sum does not overflow. That’s something that I eyeballed when reviewing, so a comment to the effect that you considered that is a good idea.

// sum will be a subset of the sum of all numbers in the range,
// so must be < (n+1)*(n/2) ≈ 500'000, which is well within the range
// of a 32-bit int.

I am not focussing on the algorithm, but more on C++ idioms.

Very good statement of what you want out of this.

I suggest you familiarize yourself with the Core Guidelines being cataloged by Bjarne Stroustrup and Herb Sutter.

#define UPPER_BOUND 1000

Don't use #define for constants or "functions" (⧺ES.31).

This is one of the first things ancient C++ brought to the table! Instead of #define with all its problems, we got const and inline.

The loop itself is sound enough, even using the prefix increment. You would need additional (non-std) libraries to write that neater using a range-for.

But, you should not write the loop at all! Stroustrup's videos will say: "Don't 'write code' — use algorithms." You want count_if, because you are asking "how many items satisfy this predicate". (update: On closer reading I see you are not counting but totaling. This is exactly the hazard of extracting the high level meaning from a low-level loop!) But again, you just have counting numbers not a collection to go through, which is not the normal case. std::generate_n will produce the list of numbers, but not in a form that can be fed to the algorithms! It pushes, while count_if pulls.

Writing the for loop, even the legacy form, is just so easy that it is hard to argue for doing it differently.

But with additional libraries: Boost.Range v2, or the upcoming Range.v3, it will be simple. Many programmers use Boost or have their own counting range (see my own tutorial and the cartoon that goes with it ☺).

So, with library in hand, I might write something like:

auto r = int_range (1, upper_bound);
auto wanted = [](int i){return (i % 3 == 0 || i % 5 == 0); }
auto sum = accumulate (r|filtered(wanted));

Since the actual work here, sum += i is so trivial that any elaboration on it makes it more code. But imagine a more realistic complex situation in business logic. The algorithm, accumulate, is named for what you are doing. The selection criteria is isolated in its own statement and named wanted, so it is easier to edit the logic later without breaking anything.

The two issues here are that the desired idioms are leading ahead of proper library/compiler support; and with a trivial program it seems artificial and convoluted to put any formal structure or “high engineering” around it.

What might be useful is to verify that your sum does not overflow. That’s something that I eyeballed when reviewing, so a comment to the effect that you considered that is a good idea.

// sum will be a subset of the sum of all numbers in the range,
// so must be < (n+1)*(n/2) ≈ 500'000, which is well within the range
// of a 32-bit int.

I am not focussing on the algorithm, but more on C++ idioms.

Very good statement of what you want out of this. Great!

I suggest you familiarize yourself with the Core Guidelines being cataloged by Bjarne Stroustrup and Herb Sutter.

#define UPPER_BOUND 1000

Don't use #define for constants or "functions" (⧺ES.31).

This is one of the first things ancient C++ brought to the table! Instead of #define with all its problems, we got const and inline.

The loop itself is sound enough, even using the prefix increment. You would need additional (non-std) libraries to write that neater using a range-for.

But, you should not write the loop at all! Stroustrup's videos will say: "Don't 'write code' — use algorithms." You want count_if, because you are asking "how many items satisfy this predicate". (update: On closer reading I see you are not counting but totaling. This is exactly the hazard of extracting the high level meaning from a low-level loop!) But again, you just have counting numbers not a collection to go through, which is not the normal case. std::generate_n will produce the list of numbers, but not in a form that can be fed to the algorithms! It pushes, while count_if pulls.

Writing the for loop, even the legacy form, is just so easy that it is hard to argue for doing it differently.

But with additional libraries: Boost.Range v2, or the upcoming Range.v3, it will be simple. Many programmers use Boost or have their own counting range (see my own tutorial and the cartoon that goes with it ☺).

So, with library in hand, I might write something like:

auto r = int_range (1, upper_bound);
auto wanted = [](int i){return (i % 3 == 0 || i % 5 == 0); }
auto sum = accumulate (r|filtered(wanted));

Since the actual work here, sum += i is so trivial that any elaboration on it makes it more code. But imagine a more realistic complex situation in business logic. The algorithm, accumulate, is named for what you are doing. The selection criteria is isolated in its own statement and named wanted, so it is easier to edit the logic later without breaking anything.

The two issues here are that the desired idioms are leading ahead of proper library/compiler support; and with a trivial program it seems artificial and convoluted to put any formal structure or “high engineering” around it.

What might be useful is to verify that your sum does not overflow. That’s something that I eyeballed when reviewing, so a comment to the effect that you considered that is a good idea.

// sum will be a subset of the sum of all numbers in the range,
// so must be < (n+1)*(n/2) ≈ 500'000, which is well within the range
// of a 32-bit int.

I am not focussing on the algorithm, but more on C++ idioms.

Very good statement of what you want out of this. Great!

I suggest you familiarize yourself with the Core Guidelines being cataloged by Bjarne Stroustrup and Herb Sutter.

#define UPPER_BOUND 1000

Don't use #define for constants or "functions" (⧺ES.31).

This is one of the first things ancient C++ brought to the table! Instead of #define with all its problems, we got const and inline.

The loop itself is sound enough, even using the prefix increment. You would need additional (non-std) libraries to write that neater using a range-for.

But, you should not write the loop at all! Stroustrup's videos will say: "Don't 'write code' — use algorithms." You want count_if, because you are asking "how many items satisfy this predicate". (update: On closer reading I see you are not counting but totaling. This is exactly the hazard of extracting the high level meaning from a low-level loop!) But again, you just have counting numbers not a collection to go through, which is not the normal case. std::generate_n will produce the list of numbers, but not in a form that can be fed to the algorithms! It pushes, while count_if pulls.

Writing the for loop, even the legacy form, is just so easy that it is hard to argue for doing it differently.

But with additional libraries: Boost.Range v2, or the upcoming Range.v3, it will be simple. Many programmers use Boost or have their own counting range (see my own tutorial and the cartoon that goes with it ☺).

So, with library in hand, I might write something like:

auto r = int_range (1, upper_bound);
auto wanted = [](int i){return (i % 3 == 0 || i % 5 == 0); }
auto sum = accumulate (r|filtered(wanted));

Since the actual work here, sum += i is so trivial that any elaboration on it makes it more code. But imagine a more realistic complex situation in business logic. The algorithm, accumulate, is named for what you are doing. The selection criteria is isolated in its own statement and named wanted, so it is easier to edit the logic later without breaking anything.

The two issues here are that the desired idioms are leading ahead of proper library/compiler support; and with a trivial program it seems artificial and convoluted to put any formal structure or “high engineering” around it.

What might be useful is to verify that your sum does not overflow. That’s something that I eyeballed when reviewing, so a comment to the effect that you considered that is a good idea.

// sum will be a subset of the sum of all numbers in the range,
// so must be < (n+1)*(n/2) ≈ 500'000, which is well within the range
// of a 32-bit int.

I am not focussing on the algorithm, but more on C++ idioms.

Very good statement of what you want out of this.

I suggest you familiarize yourself with the Core Guidelines being cataloged by Bjarne Stroustrup and Herb Sutter.

#define UPPER_BOUND 1000

Don't use #define for constants or "functions" (⧺ES.31).

This is one of the first things ancient C++ brought to the table! Instead of #define with all its problems, we got const and inline.

The loop itself is sound enough, even using the prefix increment. You would need additional (non-std) libraries to write that neater using a range-for.

But, you should not write the loop at all! Stroustrup's videos will say: "Don't 'write code' — use algorithms." You want count_if, because you are asking "how many items satisfy this predicate". (update: On closer reading I see you are not counting but totaling. This is exactly the hazard of extracting the high level meaning from a low-level loop!) But again, you just have counting numbers not a collection to go through, which is not the normal case. std::generate_n will produce the list of numbers, but not in a form that can be fed to the algorithms! It pushes, while count_if pulls.

Writing the for loop, even the legacy form, is just so easy that it is hard to argue for doing it differently.

But with additional libraries: Boost.Range v2, or the upcoming Range.v3, it will be simple. Many programmers use Boost or have their own counting range (see my own tutorial and the cartoon that goes with it ☺).

So, with library in hand, I might write something like:

auto r = int_range (1, upper_bound);
auto wanted = [](int i){return (i % 3 == 0 || i % 5 == 0); }
auto sum = accumulate (r|filtered(wanted));

Since the actual work here, sum += i is so trivial that any elaboration on it makes it more code. But imagine a more realistic complex situation in business logic. The algorithm, accumulate, is named for what you are doing. The selection criteria is isolated in its own statement and named wanted, so it is easier to edit the logic later without breaking anything.

The two issues here are that the desired idioms are leading ahead of proper library/compiler support; and with a trivial program it seems artificial and convoluted to put any formal structure or “high engineering” around it.

What might be useful is to verify that your sum does not overflow. That’s something that I eyeballed when reviewing, so a comment to the effect that you considered that is a good idea.

// sum will be a subset of the sum of all numbers in the range,
// so must be < (n+1)*(n/2) ≈ 500'000, which is well within the range
// of a 32-bit int.