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

[Diapolo]

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]

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]

@laanwj Can you take a look?

[laanwj]

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]

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]

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.

[laanwj]

Why remove the null pointer checks here?

[Diapolo]

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

[laanwj]

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]

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

[laanwj]

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.

[Diapolo]

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

[BitcoinPullTester]

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.