Use z = std::max(x - y, 0) instead of z = x - y; if (z < 0) z = 0; #9553

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:std-max changing 2 files +2 −8

practicalswift commented at 9:43 AM on January 14, 2017: contributor
Prefer ...
```
z = std::max(x - y, 0);
```
... over ...
```
z = x - y;
if (z < 0)
    z = 0;
```
... as suggested by @robmcl4.

Please note that the nSinceLastSeen is intentionally skipped since nSinceLastSeen is unused and is being removed as part of #9532.
MarcoFalke added the label Refactoring on Jan 14, 2017
practicalswift force-pushed on Jan 14, 2017
practicalswift force-pushed on Jan 14, 2017

in src/qt/coincontroldialog.cpp:None in bb355018fe outdated

 561 | @@ -562,9 +562,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
 562 |          }
 563 |  
 564 |          // after fee
 565 | -        nAfterFee = nAmount - nPayFee;
 566 | -        if (nAfterFee < 0)
 567 | -            nAfterFee = 0;
 568 | +        nAfterFee = std::max(nAmount - nPayFee, (int64_t)0);

MarcoFalke commented at 11:30 AM on January 14, 2017:

Imo this is a step back to before #4234

practicalswift commented at 12:29 PM on January 14, 2017:

Good point! Updated version pushed :-)

practicalswift force-pushed on Jan 14, 2017

in src/addrman.cpp:None in 63c8eb072f outdated

  54 | @@ -55,12 +55,10 @@ double CAddrInfo::GetChance(int64_t nNow) const
  55 |      double fChance = 1.0;
  56 |  
  57 |      int64_t nSinceLastSeen = nNow - nTime;
  58 | -    int64_t nSinceLastTry = nNow - nLastTry;
  59 | +    int64_t nSinceLastTry = std::max(nNow - nLastTry, (int64_t)0);

sipa commented at 5:35 PM on January 14, 2017:

micronit: maybe use std::max<int64_t>(nNow - nLastTry, 0) to avoid a cast?

practicalswift commented at 5:48 PM on January 14, 2017:

Good point! Fixed and pushed! :-)

in src/qt/coincontroldialog.cpp:None in 63c8eb072f outdated

 561 | @@ -562,9 +562,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
 562 |          }
 563 |  
 564 |          // after fee
 565 | -        nAfterFee = nAmount - nPayFee;
 566 | -        if (nAfterFee < 0)
 567 | -            nAfterFee = 0;
 568 | +        nAfterFee = std::max(nAmount - nPayFee, (CAmount)0);

sipa commented at 5:35 PM on January 14, 2017:

Same here.

practicalswift commented at 5:48 PM on January 14, 2017:

Fixed and pushed!

practicalswift force-pushed on Jan 14, 2017
luke-jr commented at 5:52 PM on January 14, 2017: member

I think it's actually more readable the longer way in some cases, but don't care strongly. (I would probably think differently if there was an alias called std::at_least or something.)
dcousens approved
practicalswift cross-referenced this on Jan 18, 2017 from issue Remove unused variables by practicalswift
in src/addrman.cpp:None in 660b28be38 outdated
```
  54 | @@ -55,12 +55,10 @@ double CAddrInfo::GetChance(int64_t nNow) const
  55 |      double fChance = 1.0;
  56 |  
  57 |      int64_t nSinceLastSeen = nNow - nTime;
  58 | -    int64_t nSinceLastTry = nNow - nLastTry;
  59 | +    int64_t nSinceLastTry = std::max<int64_t>(nNow - nLastTry, 0);
  60 |  
  61 |      if (nSinceLastSeen < 0)
```
jtimon commented at 10:18 PM on February 2, 2017:

why not do the same for nSinceLastSeen while you're at it for consistency? EDIT: Never mind, is being removed in https://github.com/bitcoin/bitcoin/pull/9532

practicalswift commented at 10:22 PM on February 2, 2017:

@jtimon See the comment in the PR body:

Please note that the nSinceLastSeen is intentionally skipped since nSinceLastSeen is unused and is being removed as part of #9532.

:-)
jtimon commented at 10:19 PM on February 2, 2017: contributor

ACK 660b28b ~~modulo nit.~~
in src/txmempool.cpp:None in 660b28be38 outdated
```
  54 | @@ -55,10 +55,9 @@ double
  55 |  CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const
```
MarcoFalke commented at 10:59 PM on February 2, 2017:

This is about to get removed as well, so you can drop it here.

practicalswift commented at 11:22 PM on February 2, 2017:

@MarcoFalke What should be dropped? Sorry, didn't catch that :-)

Do you mean that the changes to src/txmempool.cpp should be excluded from this commit?

MarcoFalke commented at 11:44 PM on February 2, 2017:

Everything that has "priority" in its name is scheduled to be removed, so this function as well.
practicalswift force-pushed on Feb 3, 2017
practicalswift commented at 8:22 AM on February 3, 2017: contributor

@MarcoFalke Fixed and pushed! Looks good? :-)
jtimon commented at 6:34 PM on February 3, 2017: contributor

ACK c04f80b
MarcoFalke commented at 6:47 PM on February 3, 2017: member

utACK c04f80b8f703ee3924e5999f63d127f411d73f8c
laanwj commented at 2:31 PM on February 7, 2017: member

Needs rebase after #9532
Use z = std::max(x - y, 0); instead of z = x - y; if (z < 0) z = 0; a47da4b6fe
practicalswift force-pushed on Feb 7, 2017
practicalswift commented at 3:31 PM on February 7, 2017: contributor

@laanwj Thanks for the ping! Rebased and pushed! :-)
jtimon commented at 1:51 AM on February 8, 2017: contributor

ACK a47da4b
practicalswift commented at 7:09 PM on February 14, 2017: contributor

Anything needed before merge? :-)
paveljanik commented at 7:36 PM on February 14, 2017: contributor

ACK a47da4b
laanwj merged this on Feb 15, 2017
laanwj closed this on Feb 15, 2017
laanwj referenced this in commit 4c69d683f2 on Feb 15, 2017
codablock referenced this in commit 95ab37cf2c on Jan 19, 2018
codablock referenced this in commit f2b5560083 on Jan 23, 2018
andvgal referenced this in commit 1a5fd0ee1e on Jan 6, 2019
CryptoCentric referenced this in commit 2510bfe45a on Feb 27, 2019
practicalswift deleted the branch on Apr 10, 2021
bitcoin locked this on Aug 16, 2022

Contributors

Labels

Linked (view graph)

#4234 Use a typedef for monetary values #9532 Remove unused variables