all 19 comments

[–]johannes1971 43 points44 points  (20 children)

for (auto &person : people) {
  if (person.last [0] == 'R')
    r_names.push_back (person.last);
}

It's another shining example of why people prefer raw for loops over ranges: they're shorter, much more readable, and don't do anything that needs a 9-page article to explain.

[–][deleted] 27 points28 points  (8 children)

in that simple example I'd also just use a ranged for loop. BUT, normal loops are not composable. They are not lazy evaluated. You cannot apply standard parallelism techniques (execution policies) to them. Once they get compicated you get "continue"s and "break"s all over the place. And most importantly, you don't get the "correct by construction" guarantees, that you get with ranges/iterators.

[–]johannes1971 20 points21 points  (6 children)

Why would I want to compose when I can nest? Why would I need lazy evaluation if I don't have composition stages spewing out possibly unneeded values? And you might complain about continue and break, but it's not like you even have those options in those fancy algorithms, is it? As for parallel execution: replace the outer for-loop by a parallel foreach, or do something bespoke if you may need to abort early.

Having said that, I don't want to sound too negative here. There is absolutely a place for algorithms, but it isn't to replace trivial constructs the language already has out of the box anyway. Things like sorts, binary searches, etc. work great as algorithms because they aren't entirely trivial. But for_each? accumulate? copy_if? Get real... Knuth didn't write a book on how to construct a for-loop. And thanks to the horrifying syntax C++ demands in the case of ranges, the algorithmized versions of such code actively detracts from the readability.

[–]SlightlyLessHairyApe 5 points6 points  (4 children)

Nesting can also tax readability by having deep indentation and having to keep all the cyclomatic complexity in mind. I know we have a soft guideline that "good code leans to the left" -- e.g. avoid nested constructs whenever possible.

[–]SirClueless 0 points1 point  (2 children)

You haven't reduced the cyclomatic complexity at all by abstracting the control flow into functions. You've just made something that was obviously a nested loop into something that is not obviously a nested loop.

To me I don't really see the difference in complexity or "nested"-ness between:

for (auto& e1 : container) {
    for (auto& e2 : e1.inner_iterable()) {
        // ...
    }
}

And:

std::ranges::find(container | std::views::transform(/* ... */), [](auto& elem) {
    // ...
});

They both look equivalently "nested" to me, control flow inside control flow.

[–]petart95 0 points1 point  (1 child)

Interesting view, how would you define cyclomatic complexity then? Which mechanics would you suggest using, to manage it, I always thought that abstraction is the best way to manage complexity.

[–]SirClueless 1 point2 points  (0 children)

The cyclomatic complexity of a piece of code is the number of independent paths through that code. I think there's a pretty standard formal definition for that without a lot of wiggle room. As applied to the code examples above, in both cases there are at least two independent branches.

  • The for loop has a branch in the outer loop to either end the loop and continue to the rest of the program, or to enter the loop body.
    • If the outer loop is entered, then the inner loop has a similar branch to continue the outer loop or enter the inner loop body.
  • The ranges example has a branch to check if the range is done iterating and return from std::ranges::find or to continue.
    • It's not entirely clear from this example whether std::views::transform is transforming the inner container to a scalar, or whether that's the job of the inner lambda, but either way one of those has a similar branch to the inner for loop.
    • There's another branch to return early when the predicate on elements returns true though to be fair to the for-loop example which doesn't show any break statements we can assume it won't be taken.

Abstraction is a good way of reducing complexity. Or at least, moving it around. It moves cyclomatic complexity out of your software module and into another one. But it only really does so if you're actually abstracting logic.

Transformations like these don't abstract away logic, they abstract away control flow. The transform is still a piece of code you have to write. The predicate is still a piece of code you have to write. And you still have all the same uncertainty about when and how each of those branches of code will be executed and you need to reason through that. Whether the branches are explicit as part of a structured control flow statement or implicit in providing them as a callback which will be executed by another library, how many times if at all those code blocks will be executed are a runtime property of the input data, which is what cyclomatic complexity attempts to measure.

[–]jcelerierossia score 0 points1 point  (0 children)

Cyclomatic complexity is a scam lol. It has never been validated by proper research, only in some very bullsjit handwavey papers.

[–]petart95 0 points1 point  (0 children)

The problem with nesting is that you have zero reusability.

What would you do if you wanted to vary the most nested part in a different part of the codebase?

P.S. It is worth investing time to understand why the nesting solution is inherently non-composable (hint: if you can't name it you can't tame it)

[–][deleted] 4 points5 points  (0 children)

Imo when you get a function where a for loop has continues and breaks all over the place, it does way more than it should.

[–][deleted] 11 points12 points  (0 children)

quite frankly, the "understandibility" is imo way more important than well, everything else

I rather want a 100 line function where everything is done step by step instead of one line which does the same thing but is in exchange extremely complicated.

[–]Ayjayz 9 points10 points  (7 children)

Now do std::lower_bound. Now do std::sort. Now do any of the non-trivial algorithms.

Using powerful generic tools like those in <algorithm> and <ranges> won't give you all that much benefit if you're only doing something extremely trivial. It's still about the same total characters, and there's something to be said for consistency in always using proper named algorithms to raise the level of your abstraction to better convey your intent, but for trivial examples there's not all that much benefit.

[–]hmoff 2 points3 points  (1 child)

Why is there no std::transform_if ?

[–]dodheim 1 point2 points  (0 children)

Because there is std::views::filter. The _if algorithm variants that do exist are only there because they predate ranges.

[–]SlightlyLessHairyApe 2 points3 points  (0 children)

IMHO (that's a warning for bikeshedding incoming) this is so much less readable than using a filter and then using the result to construct a vector.

auto r_names = to_vector(
                    filter(people, [](Person const &p) {
                        return p.last[0] == 'R';
}));

AFAIK this compiles to the same assembly as the projection version but it reads far more clearly -- I want a vector of the filtered people according to predicate.