Bitcoin-Qt: tweak Qt walletXXX.cpp/h code

Diapolo commented at 12:15 pm on August 2, 2013: none

WalletView:

add new signal showNormalIfMinimized()
emit the new signal in handleURI() to fix a bug, preventing the main window to show up when using bitcoin: URIs

WalletStack:

connect the showNormalIfMinimized() signal from WalletView with the showNormalIfMinimized() slot in BitcoinGUI
rework setCurrentWallet() to return a bool
add check for valid walletModel in addWallet()
add missing gui attribute initialisation in constructor

WalletFrame:

remove unused or unneded class attributes gui and clientModel
add a check for valid clientModel in setClientModel()

General:

small code formatting changes

BitcoinPullTester commented at 1:04 pm on August 2, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f6603646f33c3664c3cb3e5faac253b25450a9b1 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

Diapolo commented at 2:11 pm on August 8, 2013: none

@laanwj Can you take a look?

in src/qt/walletview.cpp: in f6603646f3 outdated

110 }
111 
112 void WalletView::setWalletModel(WalletModel *walletModel)
113 {
114     this->walletModel = walletModel;
115-    if (walletModel)

laanwj commented at 8:15 am on August 10, 2013:

Don’t remove the NULL pointer check here. It is valid to do a setWalletModel(NULL) (for example, as part of a shutdown sequence).

Diapolo commented at 11:37 am on August 10, 2013:

What about the change above in setClientModel() should we define as coding-style default that all setModel() functions should include the NULL pointer check?

BitcoinPullTester commented at 12:50 pm on August 10, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/33b1328909711d51ef1b8820fcf676256ad21776 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

in src/qt/walletview.cpp: in 33b1328909 outdated

136@@ -137,7 +137,7 @@ void WalletView::setWalletModel(WalletModel *walletModel)
137 void WalletView::incomingTransaction(const QModelIndex& parent, int start, int /*end*/)
138 {
139     // Prevent balloon-spam when initial block download is in progress
140-    if(!walletModel || !clientModel || clientModel->inInitialBlockDownload())
141+    if (clientModel->inInitialBlockDownload())

laanwj commented at 5:50 am on August 22, 2013:

Why remove the null pointer checks here?

Diapolo commented at 5:57 am on August 22, 2013:

That slot is connected in WalletView::setWalletModel(), which covers the walletModel check and WalletView::setClientModel() covers the clientModel check.

laanwj commented at 1:50 pm on August 22, 2013:

You’re correct. Although I don’t think the slot gets disconnected when a NULL model would be set later on, so it’s not completely impossible to reach here with model being NULL.

If there’s anything I’ve learned in years of writing C++ code it’s that there is no such thing as too many NULL pointer checks. Better to be safe than sorry, please just keep it :)

Diapolo commented at 6:59 am on August 23, 2013:

I’m fine with re-adding them ;-). I guess such a check is also not an expensive operation, right?

laanwj commented at 8:10 am on August 23, 2013:

Indeed, it’s not, just a 32/64 bit compare, processors can do billions of those per second. It’s also only called once per incoming transaction.

in src/qt/walletstack.cpp: in 33b1328909 outdated

35@@ -35,6 +36,10 @@ bool WalletStack::addWallet(const QString& name, WalletModel *walletModel)
36     walletView->showOutOfSyncWarning(bOutOfSync);
37     addWidget(walletView);
38     mapWalletViews[name] = walletView;
39+
40+    // Ensure a walletView is able to show the main window
41+	connect(walletView, SIGNAL(showNormalIfMinimized()), gui, SLOT(showNormalIfMinimized()));

laanwj commented at 5:50 am on August 22, 2013:

showNormalIfMinimized signal is a good idea

Diapolo commented at 7:09 am on August 23, 2013: none

Updated to revert the removal of NULL pointer checks to comply with @laanwj :).

Bitcoin-Qt: tweak Qt walletXXX.cpp/h code

WalletView:
- add new signal showNormalIfMinimized()
- emit the new signal in handleURI() to fix a bug, preventing the main
  window to show up when using bitcoin: URIs

WalletStack:
- connect the showNormalIfMinimized() signal from WalletView with the
  showNormalIfMinimized() slot in BitcoinGUI
- rework setCurrentWallet() to return a bool
- add check for valid walletModel in addWallet()
- add missing gui attribute initialisation in constructor

WalletFrame:
- remove unused or unneded class attributes gui and clientModel
- add a check for valid clientModel in setClientModel()

General:
- small code formatting changes

dbc0a6aba2

BitcoinPullTester commented at 7:53 am on August 23, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/dbc0a6aba2cf94aa1b167145a18e0b9c671aef5b for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

laanwj referenced this in commit b60012f2b6 on Aug 23, 2013

laanwj merged this on Aug 23, 2013

laanwj closed this on Aug 23, 2013

IntegralTeam referenced this in commit 22ae0bc212 on Jun 4, 2019

jonasschnelli referenced this in commit 7dbc33f617 on Nov 17, 2019

DrahtBot locked this on Sep 8, 2021

Bitcoin-Qt: tweak Qt walletXXX.cpp/h code #2872