Implement Sub/SubAssign for all Uint* types #1242
Conversation
and improve consistency across Uint* types
webmaster128
force-pushed
the
implement-sub
branch
from
ec3d339 to
4e4ccb2
Compare
There was a problem hiding this comment.
LGTM
My only suggestion - maybe .expect could include both numbers in message.
Plenty of times I stumbled upon some "anonymous" errors and had a lot of issues finding out which operation exactly failed - providing actual numbers might give some context.
Opinions?
| impl AllImpl<'_> for Uint64 {} | ||
| impl AllImpl<'_> for Uint128 {} | ||
| impl AllImpl<'_> for Uint256 {} | ||
| impl AllImpl<'_> for Uint512 {} |
You mean adding a debug message to I would do this in cases where we loop over test data where the line alone does not tell you exactly which test is failing. But we don't have this often. |
| Uint64( | ||
| self.u64() | ||
| .checked_sub(rhs.u64()) | ||
| .expect("attempt to subtract with overflow"), |
There was a problem hiding this comment.
No no, I meant strictly cases like here - but I just realized, that .expect takes &str as an argument.
So we can't do, for example, format!() here to create some nice String with local variables (at least not cheap - without any copy etc).
There was a problem hiding this comment.
Ah, good point. Yeah, this is pretty inconsistent anyways. Sometimes we just .unwrap(), sometimes we create out own message. Let's keep this in mind and address it separately.
For #1186