Page MenuHomePhabricator

Consider switching over to pre-increment rather than post-increment operator++ operator--
Open, WishlistPublic

Description

Hi guys,

I noticed the bitcoin sources inherited a rather questionable habit of using the postfix increment/decrement (VAR++/VAR--) EVERYWHERE. This is considered a rather poor habit in C++ circles and makes the code look less than awesome.

For simple types like ints this shouldn't be a problem on modern compilers because they *should* optimize away the temporary rvalue created.

However, for more complex types like iterators it can be a problem. As well as when switching out simple types for objects that implement operator++/operator--.

In general, in C++, it's always been considered best practice to use pre-increment EVERYWHERE as it's a good habit to get into.

As such, I was quite surprised when I saw that the bitcoin core developers hadn't heard of such a thing.

I would like to some day (perhaps soon, after the hard fork?), run through all the sources with a massive regex search and manually (making sure nothing breaks!) switch all instances of VAR++/VAR-- to ++VAR/--VAR whenever it's equivalent to do so.

This is in the spirit of promoting best practices in the codebase and to make bitcoin not look like amateur hour (which for the most part it isn't).

And, going forward, we should strive to always use pre-increment unless the post-increment is actually needed for the logic of the code.

So, I am creating a task for it, and I even volunteer to do it when the dust settles and no other high priority dev is going on (I await your ok to do it, basically).

Thanks!

Event Timeline

CCulianu created this task.Jul 22 2017, 21:33
CCulianu updated the task description. (Show Details)
zander added a comment.Jul 22 2017, 21:36

Thanks for subscribing me to this task.

I fully agree on this and this is a practice I follow in Bitcoin Classic. Nobody did a wholesale replace there, so don't take it to mean its perfect. Just agree on the goal.

Awesome, glad you're on-board as well. Like I said it's not a huge deal, just in general is good C++ practice. One of those cultural things, I guess, since these days the compiler will issue identical code for ints anyway.

Thanks for supporting this and good job on Classic, by the way!

It's complete cargo cult. This doesn't matter for years now, compiler will generate the same code either way if semantic is the same.

Do you have a sample case where codegen is significantly worse ?

CCulianu added a comment.EditedJul 23 2017, 15:01

This from the man who authored D370?

Your nazi braces vs. my "cargo cult".

I never said codegen is worse for ints. Read my rationale.

Stop being obtuse.

schancel removed CCulianu as the assignee of this task.Sep 28 2018, 21:24
schancel added a project: Bootcamp.
schancel removed subscribers: zander, sickpig, freetrader, CCulianu.