Use C++11 member initializer in CTxMemPoolEntry #23054

1. 
   MarcoFalke commented at 12:13 pm on September 21, 2021: member

   This removes a bunch of boilerplate, makes the code easier to read. Also, C++11 member initialization avoids accidental uninitialized members.

   Can be reviewed with the git option “–word-diff-regex=.” or with “git difftool –tool=meld”.

2. fanquake added the label Mempool on Sep 21, 2021
3. fanquake added the label Refactoring on Sep 21, 2021
4. 
   laanwj commented at 12:19 pm on September 21, 2021: member

   Code review ACK fa922062c2eb122e6c4972f20b67723332adcb8c
5. in src/txmempool.h:118 in fa922062c2 outdated

   114@@ -115,10 +115,10 @@ class CTxMemPoolEntry
   115     int64_t nSigOpCostWithAncestors;
   116 
   117 public:
   118-    CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee,
   119-                    int64_t _nTime, unsigned int _entryHeight,
   120-                    bool spendsCoinbase,
   121-                    int64_t nSigOpsCost, LockPoints lp);
   122+    CTxMemPoolEntry(const CTransactionRef& tx, const CAmount& fee,
   

   ---

   ---

   

   jonatack commented at 12:38 pm on September 21, 2021:

   Feel free to ignore but if you retouch, while changing this function declaration and definition, could also do

   0    CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee,
   

   ---

   

   MarcoFalke commented at 1:59 pm on September 21, 2021:

   Thanks, done
6. 
   jonatack commented at 12:39 pm on September 21, 2021: member

   Approach ACK
7. 
   hebasto commented at 12:39 pm on September 21, 2021: member

   Concept ACK since #17786 :)
8. in src/txmempool.cpp:40 in fa922062c2 outdated

   55+      nSizeWithDescendants{GetTxSize()},
   56+      nModFeesWithDescendants{nFee},
   57+      nCountWithAncestors{1},
   58+      nSizeWithAncestors{GetTxSize()},
   59+      nModFeesWithAncestors{nFee},
   60+      nSigOpCostWithAncestors{sigOpCost} {}
   

   ---

   ---

   

   jnewbery commented at 1:16 pm on September 21, 2021:

   Sorry for painting the bikeshed, but wouldn’t standard project style be to put the body of the function on a new line?

   0      nSigOpCostWithAncestors{sigOpCost}
   1  {}
   

   ---

   

   MarcoFalke commented at 2:00 pm on September 21, 2021:

   Clang-format accepts the trailing {}, so I think it is fine
9. in src/txmempool.cpp:35 in fa922062c2 outdated

   47+      nUsageSize{RecursiveDynamicUsage(tx)},
   48+      nTime{time},
   49+      entryHeight{entry_height},
   50+      spendsCoinbase{spends_coinbase},
   51+      sigOpCost{sigops_cost},
   52+      feeDelta{0},
   

   ---

   ---

   

   jnewbery commented at 1:17 pm on September 21, 2021:

   What do you think about initializing this (and perhaps nCountWithDescendants and nCountWithAncestors) with default initialization rather than in the initializer list?

   ---

   

   MarcoFalke commented at 1:59 pm on September 21, 2021:

   Thanks, done
10. 
    jnewbery commented at 1:17 pm on September 21, 2021: member

    Concept ACK
11. MarcoFalke force-pushed on Sep 21, 2021
12. Use C++11 member initializer in CTxMemPoolEntry

    This removes a bunch of boilerplate, makes the code easier to read.
    Also, C++11 member initialization avoids accidental uninitialized
    members.
    
    Can be reviewed with the git option "--word-diff-regex=." or with "git
    difftool --tool=meld".

    fa08d4cfb1
13. MarcoFalke force-pushed on Sep 21, 2021
14. 
    MarcoFalke commented at 2:05 pm on September 21, 2021: member

    Concept ACK since #17786 :)

    Thanks, took the other diff from there.

15. shaavan approved
16. 
    shaavan commented at 2:18 pm on September 21, 2021: contributor

    Code Review ACK fa08d4cfb1e3447a76093d46edc1c9de5ceee454

    This PR simplifies the code, removing unnecessary parts, at the same time, keeping the same functionality. The involved constants are now initialized through default initialization rather than from the function. The Clang Formatting of the PR is correct. So there are no formatting issues either. Thanks, @MarcoFalke, for catching these redundancies.

17. 
    jnewbery commented at 2:42 pm on September 21, 2021: member

    Code review ACK fa08d4cfb1e3447a76093d46edc1c9de5ceee454
18. 
    MarcoFalke commented at 5:10 pm on September 21, 2021: member

    Failing CI can be ignored
19. in src/txmempool.h:109 in fa08d4cfb1

    107     uint64_t nSizeWithDescendants;   //!< ... and size
    108     CAmount nModFeesWithDescendants; //!< ... and total fees (all including us)
    109 
    110     // Analogous statistics for ancestor transactions
    111-    uint64_t nCountWithAncestors;
    112+    uint64_t nCountWithAncestors{1};
    

    ---

    ---

    

    JeremyRubin commented at 5:29 pm on September 21, 2021:

    I don’t see a reason to not use the initializer list for these fields, seems more consistent style wise.

    ---

    

    MarcoFalke commented at 6:42 pm on September 21, 2021:

    I changed this because several people asked me to, see #23054 (review)
20. fanquake merged this on Sep 23, 2021
21. fanquake closed this on Sep 23, 2021

    ---

22. MarcoFalke deleted the branch on Sep 23, 2021
23. sidhujag referenced this in commit 0cd8578262 on Sep 24, 2021
24. 
    hebasto commented at 5:15 pm on September 24, 2021: member

    Post-merge ACK fa08d4cfb1e3447a76093d46edc1c9de5ceee454.
25. Fabcien referenced this in commit 9731bc95ab on Oct 7, 2022
26. DrahtBot locked this on Oct 30, 2022

Contributors
MarcoFalke laanwj jonatack hebasto jnewbery shaavan JeremyRubin

Labels
Refactoring Mempool