Refactor: Changes postincrement to preincrement for iterator in for loops in src/wallet files #14753

Changed the postincrement operator to a preincrement operator for the iterator in the for loops in src/wallet/rpcdump.cpp and src/wallet/wallet.cpp.

For integers the compiler does the optimisation automatically. In this case an iterator is used, so this changes avoid creating an unnecessary temporary copy of the iterator.

Comments on this PR suggest the use of a preincrement for optimisation.

Targeted only the files in src/wallet to not overcomplicate things and to keep this changes isolated to the scr/wallet files, so it can be easier to review and test.

I look forward to your opinions about this.

Thank you!

I would be pretty surprised if this difference isn't optimized out.

@gmaxwell surprisingly this SO answer suggests that's not true https://stackoverflow.com/a/38948073/8253658

#include <stdio.h>
#include <stdint.h>
#include <vector>

extern std::vector<int64_t> tv;
int main(void){
  for (std::vector<int64_t>::const_iterator it = tv.begin(); it != tv.end(); it++) {
    printf("bloop.\n");
  }
}

Gives the same object code for me when compiled with g++ 7.3.1 with either it++ or ++it. Anyone care to improve my test?

It might be better to use range-based for loop that doesn't have to call end() every time. (Maybe it's already optimized by the compiler in -O2) Anyhow, it would not be a really long code anymore.

None of these things matter at all.

The suggestion for using preincrement over postincrement is because in some cases, one can avoid creating a copy. In practice, for all simple types this does not matter, including most standard library iterators (which just encapsulate a single pointer).

It's a good guideline, but there is no need to go over the code and change things everywhere (the developer guidelines even point out that you shouldn't make PRs that just change style). If you believe this is not just a stylistic change, please demonstrate that there is an observable performance benefit (by adding a benchmark).

These small changes can indeed be considered not that important. To go and apply them everywhere now, might not make much sense, indeed.

Thank you all for your input!