I prefer the original code...

@vondele: Please rebase

@zamar: done.

@vondele

I would say that the new code doesn't feel like a simplification to me, but rather like a complexification.

Each introduction of a boolean parameter in a function introduce a readability headache, because it means the function has two execution paths which complicates the analysis (both at caller and callee points).

The old update_stats() function was clean and branchless, but now your new update-all-stats() introduces two(!) new boolean parameters, which means that it has four new execution paths... It is always a bad sign when a function has eight parameters.

See chapter 5 ("Avoid Candy Interfaces") of Steve Maguire's "Writing Solid Code":
http://cs.brown.edu/courses/cs190/2008/documents/restricted/Writing%20Solid%20Code.pdf

I agree with @snicolet
The original code is much easier to work on.

@snicolet, @VoyagerOne, in addition to what I wrote before, the motivation for this patch is to make search<>() more easy to read, to avoid code duplication, and to make the code that updates the stats more localized in search.cpp. In more detail:

• search<>() is a key and very non-trivial function, and has become very long. In master, the single function is 611 lines. The two locations that update the stats are, within the same function, 500 lines apart. The patch reduces the length of search<>() by 34 lines. This improves readability.

• It does so by merging code that is a literal copy and paste (including comments):
  https://github.com/official-stockfish/Stockfish/blob/master/src/search.cpp#L628-L630
  https://github.com/official-stockfish/Stockfish/blob/master/src/search.cpp#L1114-L1116
  or otherwise very similar. As a result the overall reduction of code is about 10 lines.

• The number of if clauses in the source is reduced from 11 to 8 (counting in deleted and added lines), and in no case more if statements are executed at runtime (thanks templates..)

• The code separates concerns in that search<>() focuses more on search, while all stat update code is in one place that roughly fits one 'modern screen' (55 lines is still a bit).

The number of arguments to update_all_stats is related to what updating the stats requires currently. It seemingly is rather complex, and that's the input the corresponding function needs. While the original form of the code might allow for an easier hack, the current form facilitates IMO better design. As I said in the original submission, I hope that this form inspires further simplifications.

Results for 20 tests for each version:

        Base      Test      Diff      
Mean    1625914   1633342   -7428     
StDev   12159     11641     8524      

p-value: 0,808
speedup: 0,005

I kind of see the point of this patch, but for me the new version is more difficult to read and understand than the original one...

So I have to reject this patch.