Prefer initialization to assignment in constructors. Prefer in-class initializers to member initializers in constructors for constant initializers. #13766

1. 
   practicalswift commented at 2:09 pm on July 26, 2018: contributor

   Initialize members in initialization lists. Prefer in-class initializers to member initializers in constructors for constant initializers.

   Rationale:

   • C.49: Prefer initialization to assignment in constructors
   • C.48: Prefer in-class initializers to member initializers in constructors for constant initializers
2. fanquake added the label Refactoring on Jul 26, 2018
3. 
   DrahtBot commented at 2:54 pm on July 26, 2018: member

   • #14046 (net: Refactor message parsing (CNetMessage), adds flexibility by jonasschnelli)
   • #13123 (net: Add Clang thread safety annotations for guarded variables in the networking code by practicalswift)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. in src/primitives/transaction.cpp:21 in b61c902ae6 outdated

   21-    scriptSig = scriptSigIn;
   22-    nSequence = nSequenceIn;
   23 }
   24 
   25-CTxIn::CTxIn(uint256 hashPrevTx, uint32_t nOut, CScript scriptSigIn, uint32_t nSequenceIn)
   26+CTxIn::CTxIn(uint256 hashPrevTx, uint32_t nOut, CScript scriptSigIn, uint32_t nSequenceIn) : prevout(COutPoint(hashPrevTx, nOut)), scriptSig(scriptSigIn), nSequence(nSequenceIn)
   

   ---

   ---

   

   MarcoFalke commented at 3:53 pm on July 26, 2018:

   Could put a \n before the : to avoid having excessively long lines, no? (Feddback also applies to other places with long lines)
5. practicalswift force-pushed on Jul 26, 2018
6. 
   practicalswift commented at 4:21 pm on July 26, 2018: contributor

   @MarcoFalke Good point! Newlines added. Please re-review :-)
7. 
   promag commented at 5:21 pm on July 26, 2018: member

   Concept ACK.
8. in src/wallet/wallet.cpp:4343 in 5e80e9345c outdated

   4356+CKeyPool::CKeyPool(const CPubKey& vchPubKeyIn, bool internalIn) : vchPubKey(vchPubKeyIn), fInternal(internalIn)
   4357 {
   4358     nTime = GetTime();
   4359-    vchPubKey = vchPubKeyIn;
   4360-    fInternal = internalIn;
   4361     m_pre_split = false;
   

   ---

   ---

   

   Empact commented at 9:38 pm on July 26, 2018:

   nit: this one too?
9. in src/script/standard.cpp:260 in 5e80e9345c outdated

   255@@ -256,7 +256,8 @@ class CScriptVisitor : public boost::static_visitor<bool>
   256 private:
   257     CScript *script;
   258 public:
   259-    explicit CScriptVisitor(CScript *scriptin) { script = scriptin; }
   260+    explicit CScriptVisitor(CScript *scriptin) : script(scriptin) {
   261+    }
   

   ---

   ---

   

   Empact commented at 9:39 pm on July 26, 2018:

   nit: whitespace

   ---

   

   practicalswift commented at 10:01 pm on July 26, 2018:

   @Empact Where do you want the whitespace added/removed? :-)

   ---

   

   MarcoFalke commented at 10:04 pm on July 26, 2018:

   Probably leave everything in a single line?

   ---

   

   Empact commented at 10:09 pm on July 26, 2018:

   clang-format-diff.py accepts at least:

   0explicit CScriptVisitor(CScript* scriptin) : script(scriptin) {}
   1explicit CScriptVisitor(CScript* scriptin) : script(scriptin)
   2{
   3}
   

   For an empty set, like Marco I like the former.

   ---

   

   Empact commented at 10:12 pm on July 26, 2018:

   Could also apply to CompareInvMempoolOrder, FreespaceChecker, etc.
10. practicalswift force-pushed on Jul 26, 2018
11. practicalswift force-pushed on Aug 2, 2018
12. practicalswift force-pushed on Aug 2, 2018
13. practicalswift force-pushed on Aug 2, 2018
14. practicalswift force-pushed on Aug 2, 2018
15. 
    practicalswift commented at 11:57 am on August 2, 2018: contributor

    @Empact Thanks for reviewing. Feedback addressed. Please re-review :-)
16. 
    laanwj commented at 12:27 pm on August 29, 2018: member

    Some of these initializers (those with constant values) could move to the class declaration itself
17. practicalswift force-pushed on Aug 29, 2018
18. practicalswift force-pushed on Aug 29, 2018
19. practicalswift force-pushed on Aug 30, 2018
20. practicalswift force-pushed on Aug 30, 2018
21. practicalswift renamed this:
    Initialize members in initialization lists
    Prefer initialization to assignment in constructors. Prefer in-class initializers to member initializers in constructors for constant initializers.
    on Aug 30, 2018
22. practicalswift force-pushed on Aug 30, 2018
23. 
    practicalswift commented at 9:41 am on August 30, 2018: contributor

    @laanwj Now preferring in-class initializers to member initializers in constructors for constant initializers. Please re-review the PR.

    Please note that a redundant call to filterInventoryKnown.reset() has been removed. That looks correct, right?

24. practicalswift force-pushed on Aug 30, 2018
25. 
    practicalswift commented at 9:53 am on August 30, 2018: contributor

    Also added a developer note: “Prefer initialization to assignment in constructors”
26. Prefer initialization to assignment in constructors 6a93312c88
27. Prefer in-class initializers to member initializers in constructors for constant initializers 3d7db2b35d
28. Remove redundant filterInventoryKnown.reset() a7d8a96fd3
29. Add developer note: Prefer initialization to assignment in constructors 9e65bd423e
30. Pass uint256 hashPrevTx as const reference 157ab566a6
31. practicalswift force-pushed on Sep 5, 2018
32. practicalswift closed this on Oct 22, 2018

    ---

33. practicalswift deleted the branch on Apr 10, 2021
34. DrahtBot locked this on Aug 18, 2022

Contributors
practicalswift DrahtBot MarcoFalke promag Empact laanwj

Labels
Refactoring