fix: add display_currency_decimal method #3445
Conversation
|
The Or how about removing |
therustmonk
force-pushed
the
display-currency-improved
branch
from
84e5ef8 to
36e9d8b
Compare
therustmonk
force-pushed
the
display-currency-improved
branch
from
36e9d8b to
7510b2e
Compare
|
|
||
| newtype_ops! { [Tari] {add sub} {:=} Self Self } | ||
| newtype_ops! { [Tari] {mul div rem} {:=} Self f64 } | ||
| #[derive( |
There was a problem hiding this comment.
newtype-ops looks unmaintained. How about using derive_more instead?
There was a problem hiding this comment.
Yeah if we're already using derive_more now and it's easy to remove newtype_ops I think that's fine
|
It's also possible to remove pub struct TariFormat {
MicroTari,
Tari,
Auto,
}
pub struct FormattedTari {
format: TariFormat,
separator: Option<char>,
}
impl MicroTari {
pub fn formatted(&self, format: TariFormat, separator: Option<char>) -> FormattedTari {
FormattedTari {
format,
separator,
}
}
}
impl Display for FormattedTari { /* etc. */ }What do you think? |
| #[error("Tari value convert error `{0}`")] | ||
| TariConvertError(#[from] TariConvertError), |
There was a problem hiding this comment.
| #[error("Tari value convert error `{0}`")] | |
| TariConvertError(#[from] TariConvertError), | |
| #[error("Tari value conversion error `{0}`")] | |
| TariConversionError(#[from] TariConversionError), |
| Err(MicroTariError::ParseError("value cannot be negative".to_string())) | ||
| } else { | ||
| Ok(MicroTari::from(Tari::from(v.max(0.0)))) | ||
| // TODO: Check. It can't be `NaN` anymore. Still we need `.max(0.0)` check? |
There was a problem hiding this comment.
is this comment still current?
There was a problem hiding this comment.
No idea, why we compare the unsigned value with 0.0 🤷♂️ I left the comment to double-check it together. How about removing that max call?
There was a problem hiding this comment.
It looks like you did remove the max? Which is why I ask :)
There was a problem hiding this comment.
Ah, right, it's another one. I've removed it, because if a value is not less than 0.0 (greater or equal 0.0) it's always a win in max(0.0) comparison. I guess we had it to handle the f64::NaN case, but Decimal doesn't support NaN values at all and that comparison doesn't make sense anymore. If I'm correct about why we had that max(0.0) call.
There was a problem hiding this comment.
| impl From<Tari> for MicroTari { | ||
| fn from(v: Tari) -> Self { | ||
| MicroTari((v.0 * 1e6) as u64) | ||
| MicroTari((v.0 * 1_000_000u32).try_into().unwrap()) |
There was a problem hiding this comment.
In the past, the value could be truncated silently, I thought it's better to panic here, but probably even better to replace the trait to TryFrom.
|
|
||
| newtype_ops! { [Tari] {add sub} {:=} Self Self } | ||
| newtype_ops! { [Tari] {mul div rem} {:=} Self f64 } | ||
| #[derive( |
There was a problem hiding this comment.
Yeah if we're already using derive_more now and it's easy to remove newtype_ops I think that's fine
| v.0 | ||
| } | ||
| } | ||
| pub type TariConvertError = DecimalConvertError; |
There was a problem hiding this comment.
| pub type TariConvertError = DecimalConvertError; | |
| pub type TariConversionError = DecimalConvertError; |
* development: fix: add display_currency_decimal method (tari-project#3445) fix: add sanity checks to prepare_new_block (tari-project#3448) test: profile wallet sqlite database operations (tari-project#3451) test: create cucumber test directory if necessary (tari-project#3452) chore: improve cucumber tags and run time speed (tari-project#3439)
Description
The inner type of
Tarichanged fromf64todecimal_rs::Decimal.Also,
display_currencymethod was replaced withformat_currencythat adds separators to formatted strings.Motivation and Context
display_currencymethod forces us to usef64that is not accurate for finance calculations.How Has This Been Tested?
Updated unit tests.
TODO: Smoke test: run the wallet and the node.