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

pull practicalswift wants to merge 5 commits into bitcoin:master from practicalswift:initialize-members-in-initialization-list changing 15 files +87 −164
  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:

  2. fanquake added the label Refactoring on Jul 26, 2018
  3. DrahtBot commented at 2:54 PM on July 26, 2018: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->Reviewers, this pull request conflicts with the following ones:

    • #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:

    explicit CScriptVisitor(CScript* scriptin) : script(scriptin) {}
explicit CScriptVisitor(CScript* scriptin) : script(scriptin)
{
}

    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. DrahtBot cross-referenced this on Aug 23, 2018 from issue Add p2p layer encryption with ECDH/ChaCha20Poly1305 by jonasschnelli
  17. 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

  18. practicalswift force-pushed on Aug 29, 2018
  19. DrahtBot cross-referenced this on Aug 29, 2018 from issue net: Refactor message parsing (CNetMessage), adds flexibility by jonasschnelli
  20. practicalswift force-pushed on Aug 29, 2018
  21. practicalswift force-pushed on Aug 30, 2018
  22. practicalswift force-pushed on Aug 30, 2018
  23. 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
  24. practicalswift force-pushed on Aug 30, 2018
  25. 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?

  26. practicalswift force-pushed on Aug 30, 2018
  27. practicalswift commented at 9:53 AM on August 30, 2018: contributor

    Also added a developer note: "Prefer initialization to assignment in constructors"

  28. practicalswift cross-referenced this on Aug 31, 2018 from issue wallet: Remove unused local variable old_label by practicalswift
  29. DrahtBot cross-referenced this on Aug 31, 2018 from issue Minor style enhancement in documentation by fedsten
  30. Prefer initialization to assignment in constructors 6a93312c88
  31. Prefer in-class initializers to member initializers in constructors for constant initializers 3d7db2b35d
  32. Remove redundant filterInventoryKnown.reset() a7d8a96fd3
  33. Add developer note: Prefer initialization to assignment in constructors 9e65bd423e
  34. Pass uint256 hashPrevTx as const reference 157ab566a6
  35. practicalswift force-pushed on Sep 5, 2018
  36. DrahtBot cross-referenced this on Sep 20, 2018 from issue net: Add Clang thread safety annotations for guarded variables in the networking code by practicalswift
  37. DrahtBot cross-referenced this on Sep 21, 2018 from issue Free BerkeleyEnvironment instances when not in use by ryanofsky
  38. practicalswift closed this on Oct 22, 2018
  39. practicalswift deleted the branch on Apr 10, 2021
  40. bitcoin locked this on Aug 18, 2022
Contributors
practicalswiftDrahtBotMarcoFalkepromagEmpactlaanwj
Labels
Refactoring
Linked (view graph)
#14117 wallet: Remove unused local variable old_label
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:55 UTC