[xfs]

I love C++17 fold expression, but...

Local consts are not particularly useful, especially for things like ints. Compilers know it's const, and a reader doesn't need to worry about it in a local scope.

width, height usually come in pairs, so putting them on the same line is usual.

nPixels and nBytes are not used more than once so they are not useful abstractions, and the pattern of `width * height * bytes_per_pixel` is so common that there is no ambiguity about what it does.

The idx lambda is probably a distracting abstraction which requires the reader to think of it in a different context than the immediate what pixel goes to where. Again, the pixel moving pattern in the right is so common it's familiar to most people working in similar areas, and in terms of error-prone both are not better than the other.

[danielscrubs]

When I read that it's const I can in my mind "drop it". It's there, I know where the value was set and don't have to skim any longer for updates. It isn't for the compiler, its for me, the next guy who has to read your code.

Same thing with doing two separate things in the loop. Not only can it stop the the compiler from optimisations from time to time (the c++ compiler is very clever... but many times it gives up), I've had speed increases in breaking it up into several loops, but I also have to search and in my mind break things up. Very easy to do when I'm writing the code (and I'd probably do the same because of laziness), but quite annoying when I read someone else's code that does it.

[xfs]

If you as a reader need const to make sure a local variable is not changed this is usually a symptom of this function being too long to see at a glance. And quite often as code changes I do need to make some local variable non-const, and it quickly becomes annoying to fiddle back and forth enforcing const-ness of every local variable.

The argument about optimization is almost certainly premature optimization. Most of the time how you write a loop doesn't matter. You only find out what matters via profiling and refactor accordingly.

[hhmc]

> If you as a reader need const to make sure a local variable is not changed this is usually a symptom of this function being too long to see at a glance

If the compiler can enfore an invariant (constness), why cede that functionality on the hopes you can ensure it yourself?

By the same line of argumentation you should do away with the static type system.

[gintery]

Most people hate const at first (myself included) but once you get used to it, it really does reduces mental load. Not having to glance around is precisely the point, its not a big thing but it does help.

[xfs]

I stopped using it for locals after it increased my mental load. The maintenance cost of const for all local variables is huge during refactor like you're fighting it just to get things done. And in most cases where the type of a variable matters I do have to glance around like when I need to remove a variable or change its type or refactor its dependent variables so const doesn't really for those cases.

[gintery]

If you have to assign to a const variable during a small refactor, then maybe it shouldn't have been const in the first place? I'm struggling to imagine examples where this is a real problem

[danielscrubs]

So you prioritise the writing of the code instead of the reading.

In that case, it makes sense.

[dkersten]

> Compilers know it's const

I can still assign to it. Sure, you can argue that if the code is complex enough that you might accidentally do that without it being obvious, then the code should be simplified or split up, but that's besides the point. If its const, the compiler will complain, if its not, it will silently let me. Nobody can write ideally-factored code all of the time and following good consistent practices helps. Also, I hear arguments like that all the time, but so much of actual real-world shipped code breaks these "rules", so I'd rather be pragmatic and choose a style that improves otherwise imperfect code.

Its also as much a hint to future me that the variable is intended to not be modified.

[hhmc]

Regardless of the induvidual merits of the changes (and I do disagree with you), it's not reasonable to say: "Folds are overcomplicated, here's a version without folds which is simpler" whilst also making a range of unrelated changes to reduce code size.

[xfs]

Folds are not complicated. They are just not familiar to the intended readers.

The changed version is idiomatic in image processing or similar areas that deal with pixels. Being idiomatic makes it familiar to read, and easy to change.

[hhmc]

But the wider context here -- and the reason the changed version was presumably written -- is the accusation that folds are overcomplicated.

It's under that lens that I am criticizing the changed code.

[sharpneli]

Not just folds. Also the lambda and the inner loop which is memcpy instead of range3. It’s about every part of the function, including but not limited to the folds.