qt: Initialize non-static class members that were previously neither initialized where defined nor in constructor #12928

practicalswift commented at 11:25 PM on April 9, 2018: contributor

Initialize variables previously neither defined where defined nor in constructor:

editStatus
autoCompleter

Also; initialize non-static class members where they are defined in accordance with developer notes.

Initialize editStatus and autoCompleter. Previously not initialized where defined or in constructor. 73bc1b7cd2

Initialize non-static class members where they are defined f131872653

fanquake added the label GUI on Apr 9, 2018

practicalswift renamed this:
~~qt: Initialize variables previously neither defined where defined nor in constructor~~
qt: Initialize variables previously neither initialized where defined nor in constructor
on Apr 9, 2018

practicalswift renamed this:
~~qt: Initialize variables previously neither initialized where defined nor in constructor~~
qt: Initialize variables that were previously neither initialized where defined nor in constructor
on Apr 9, 2018

practicalswift renamed this:
~~qt: Initialize variables that were previously neither initialized where defined nor in constructor~~
qt: Initialize non-static class members that were previously neither initialized where defined nor in constructor
on Apr 9, 2018

in src/qt/addresstablemodel.h:84 in f131872653 outdated

  80 | @@ -81,10 +81,10 @@ class AddressTableModel : public QAbstractTableModel
  81 |      OutputType GetDefaultAddressType() const;
  82 |  
  83 |  private:
  84 | -    WalletModel *walletModel;
  85 | -    AddressTablePriv *priv;
  86 | +    WalletModel *walletModel = nullptr;

promag commented at 1:30 AM on April 10, 2018:

walletModel and priv are initialized in constructor: https://github.com/bitcoin/bitcoin/blob/727175a08dffcb5f18f8faac562ab54f89e5f36e/src/qt/addresstablemodel.cpp#L162

practicalswift commented at 4:13 AM on April 10, 2018:

@promag See the pull request description – it was only editStatus and autoCompleter that were previously neither initialized where defined nor in constructor. The other ones:

Also; initialize non-static class members where they are defined in accordance with developer notes.

promag commented at 10:38 AM on May 2, 2018:

walletModel is initialized in constructor, why this? Maybe change here to

WalletModel* const walletModel;

so it only compiles if initialized in constructor.

in src/qt/rpcconsole.h:148 in f131872653 outdated

 144 | @@ -145,18 +145,18 @@ public Q_SLOTS:
 145 |      };
 146 |  
 147 |      interfaces::Node& m_node;
 148 | -    Ui::RPCConsole *ui;
 149 | -    ClientModel *clientModel;
 150 | +    Ui::RPCConsole *ui = nullptr;

promag commented at 1:55 AM on April 10, 2018:

Same as above: https://github.com/bitcoin/bitcoin/blob/727175a08dffcb5f18f8faac562ab54f89e5f36e/src/qt/rpcconsole.cpp#L454-L463

practicalswift commented at 4:13 AM on April 10, 2018:

@promag See the pull request description – it was only editStatus and autoCompleter that were previously neither initialized where defined nor in constructor. The other ones:

Also; initialize non-static class members where they are defined in accordance with developer notes.

promag commented at 10:42 AM on May 2, 2018:

Same as above

Ui::RPCConsole* const ui;

jonasschnelli commented at 6:19 PM on April 10, 2018: contributor

Makes sense and code looks better afterwards. utACK f131872653dadafd9af8bec255dfd2bddd75a471

MarcoFalke commented at 6:31 PM on April 10, 2018: member

In which case you can remove the redundant initialization from the constructor, no?

promag commented at 10:50 PM on April 10, 2018: member

I meant to say what @MarcoFalke asked.

practicalswift commented at 6:11 AM on April 11, 2018: contributor

@promag @MarcoFalke Is this the suggested change? :-)

diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp
index 1e3acd7..b6ecd40 100644
--- a/src/qt/addresstablemodel.cpp
+++ b/src/qt/addresstablemodel.cpp
@@ -159,7 +159,7 @@ public:
 };

 AddressTableModel::AddressTableModel(WalletModel *parent) :
-    QAbstractTableModel(parent),walletModel(parent),priv(0)
+    QAbstractTableModel(parent), walletModel(parent)
 {
     columns << tr("Label") << tr("Address");
     priv = new AddressTablePriv(this);
diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index 5122bab..7924840 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -455,12 +455,7 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty
     QWidget(parent),
     m_node(node),
     ui(new Ui::RPCConsole),
-    clientModel(0),
-    historyPtr(0),
-    platformStyle(_platformStyle),
-    peersTableContextMenu(0),
-    banTableContextMenu(0),
-    consoleFontSize(0)
+    platformStyle(_platformStyle)
 {
     ui->setupUi(this);
     QSettings settings;

promag commented at 6:35 AM on April 11, 2018: member

Yes.

Remove redundant initializations from the constructor 1e7813e9bb

practicalswift commented at 10:33 AM on April 11, 2018: contributor

Updated. Now including @MarcoFalke and @promag's suggestions. Please re-review :-)

practicalswift commented at 10:26 AM on May 2, 2018: contributor

Anyone willing to re-review? @laanwj, would you mind reviewing? :-)

Feels icky to have uninitialized variables laying around :-)

in src/qt/rpcconsole.h:154 in 1e7813e9bb outdated

 158 | -    RPCTimerInterface *rpcTimerInterface;
 159 | -    QMenu *peersTableContextMenu;
 160 | -    QMenu *banTableContextMenu;
 161 | -    int consoleFontSize;
 162 | -    QCompleter *autoCompleter;
 163 | +    const PlatformStyle *platformStyle = nullptr;

promag commented at 10:42 AM on May 2, 2018:

Same as above

const PlatformStyle* const platformStyle;

promag commented at 10:46 AM on May 2, 2018: member

IMO if a member is initialized in the constructor with a non const expression then it should not be initialized where it's defined.

Make sure initialization occurs in the constructor 3fdc5fee18

practicalswift commented at 11:57 AM on May 2, 2018: contributor

@promag Good point. Updated – please re-review! :-)

promag commented at 12:12 PM on May 2, 2018: member

utACK 3fdc5fe.

laanwj commented at 2:49 PM on May 2, 2018: member

utACK 3fdc5fee1864c759bceabaa61e104f954b9d1180

laanwj merged this on May 2, 2018

laanwj closed this on May 2, 2018

laanwj referenced this in commit ef006d9284 on May 2, 2018

jasonbcox referenced this in commit 4dc32c990a on Sep 13, 2019

jonspock referenced this in commit 218b5e4d2a on Dec 22, 2019

proteanx referenced this in commit 05dbc5ffaf on Dec 23, 2019

practicalswift deleted the branch on Apr 10, 2021

UdjinM6 referenced this in commit 1a0277da6f on May 21, 2021

UdjinM6 referenced this in commit 6423395e2e on May 25, 2021

TheArbitrator referenced this in commit a92276dede on Jun 4, 2021

gades referenced this in commit 1e1083bc29 on May 9, 2022

DrahtBot locked this on Aug 18, 2022