wallet, refactor: Remove duplicate map lookups in GetAddressBalances #19828

pull promag wants to merge 1 commits into bitcoin:master from promag:2020-08-getaddressbalances changing 1 files +0 −3

promag commented at 12:03 PM on August 28, 2020: member

Now just one lookup in balances instead of three.
fanquake added the label Wallet on Aug 28, 2020
in src/wallet/wallet.cpp:3376 in 30678be9cb outdated
```
3371 | @@ -3372,10 +3372,8 @@ std::map<CTxDestination, CAmount> CWallet::GetAddressBalances() const
3372 |                      continue;
3373 |  
3374 |                  CAmount n = IsSpent(walletEntry.first, i) ? 0 : wtx.tx->vout[i].nValue;
3375 | -
3376 | -                if (!balances.count(addr))
3377 | -                    balances[addr] = 0;
3378 | -                balances[addr] += n;
3379 | +                auto ret = balances.emplace(addr, n);
3380 | +                if (!ret.second && n > 0) ret.first->second += n;
```
laanwj commented at 3:38 PM on August 28, 2020:

I think it's nice to avoid a triple lookup here, but, I think the old code was much more readable. Isn't there a clearer way to write this which keeps it clear that "n is always added to the balance, which starts at 0"?

Wouldn't this simply work?:

balances.emplace(addr, 0).first->second += n;
promag commented at 3:53 PM on August 28, 2020:

Nice one, will use that if you don't mind.

sipa commented at 3:55 PM on August 28, 2020:

Why not balances[addr] += n ?

promag commented at 3:56 PM on August 28, 2020:

Done, thanks!

theStack commented at 3:59 PM on August 28, 2020:

Why not balances[addr] += n ?

Heh, I also thought about that, but then had doubts that integral types (without ctor) also get initialized. Seems I was wrong: "When invoking operator[] and the key is missing, the value is initialized using the expression mapped_type() which is the default constructor for class types, and zero initialization for integral types." https://stackoverflow.com/a/38549094

promag commented at 4:01 PM on August 28, 2020:

🤦

Done!
promag force-pushed on Aug 28, 2020
wallet, refactor: Remove duplicate map lookups in GetAddressBalances b35e74ba37
promag force-pushed on Aug 28, 2020
theStack commented at 4:03 PM on August 28, 2020: member

ACK b35e74ba379bdc12ea6d49a45469f0d6aa74cc27
achow101 commented at 5:31 PM on August 28, 2020: member

ACK b35e74ba379bdc12ea6d49a45469f0d6aa74cc27

Nice and simple
practicalswift commented at 6:53 AM on August 30, 2020: contributor

ACK b35e74ba379bdc12ea6d49a45469f0d6aa74cc27
fanquake merged this on Aug 31, 2020
fanquake closed this on Aug 31, 2020
sidhujag referenced this in commit dac89b2e22 on Aug 31, 2020
Fabcien referenced this in commit 9f6318190b on Sep 14, 2021
DrahtBot locked this on Feb 15, 2022

Contributors

Labels