Updated FormatMoneyUseCase #3657
Conversation
shamim-emon
force-pushed
the
fix-issue-3642
branch
from
9d56c95
to
0b8d70d
Compare
| * @return The formatted string representation of the value. | ||
| */ | ||
| suspend fun format(value: Double, shortenAmount: Boolean, isCrypto: Boolean = false): String { | ||
| val result = if (isCrypto) { |
There was a problem hiding this comment.
You can directly return, no need for result var
| private fun formatCrypto(value: Double): String { | ||
| val result = cryptoFormatter.format(value) | ||
| return when { | ||
| result.lastOrNull() == localDecimalSeparator().firstOrNull() -> { |
There was a problem hiding this comment.
I don't like this logic - can we just fix the formatter?
There was a problem hiding this comment.
Removed the function, just relying on the suggested formatter now
| @@ -23,25 +32,69 @@ class FormatMoneyUseCase @Inject constructor( | |||
| private val withoutDecimalFormatter = DecimalFormat("###,###", DecimalFormatSymbols(locale)) | |||
| private val withDecimalFormatter = DecimalFormat("###,###.00", DecimalFormatSymbols(locale)) | |||
| private val shortenAmountFormatter = DecimalFormat("###,###.##", DecimalFormatSymbols(locale)) | |||
| private val cryptoFormatter = | |||
| DecimalFormat("###,###,##0.${"0".repeat(9)}", DecimalFormatSymbols(locale)) | |||
There was a problem hiding this comment.
Can you try fixing the formatter by "#".repeat(9)
| shortenAmount = false, | ||
| isCrypto = true, | ||
| locale = Locale.ENGLISH, | ||
| expectedOutput = "123,456.000000000" |
There was a problem hiding this comment.
We don't expect such zeros
There was a problem hiding this comment.
Updated the result as-per the suggested formatter expected result
| @@ -23,25 +32,35 @@ class FormatMoneyUseCase @Inject constructor( | |||
| private val withoutDecimalFormatter = DecimalFormat("###,###", DecimalFormatSymbols(locale)) | |||
| private val withDecimalFormatter = DecimalFormat("###,###.00", DecimalFormatSymbols(locale)) | |||
| private val shortenAmountFormatter = DecimalFormat("###,###.##", DecimalFormatSymbols(locale)) | |||
| private val cryptoFormatter = DecimalFormat("#".repeat(9), DecimalFormatSymbols(locale)) | |||
There was a problem hiding this comment.
Ah, I mean we need to support showing any arbitrary crypto with up to 9 decimals precision without showing unnecessary trailing digits. The correct pattern should be:
"###,###." + "#".repeat(9)
| locale = Locale.ENGLISH, | ||
| expectedOutput = "123456" | ||
| ), | ||
| GERMAN_SHOW_DECIMAL_CRYPTO( |
There was a problem hiding this comment.
We need a tear case for "0.000345 BTC" for example
| @@ -56,7 +56,7 @@ import com.ivy.navigation.navigation | |||
| import com.ivy.navigation.screenScopedViewModel | |||
| import com.ivy.ui.R | |||
| import com.ivy.wallet.domain.data.CustomExchangeRateState | |||
| import com.ivy.wallet.domain.data.IvyCurrency | |||
| import com.ivy.legacy.domain.data.IvyCurrency | |||
There was a problem hiding this comment.
What caused this package change?
There was a problem hiding this comment.
Package directive of IvyCurrency data class was wrong. It resides in com.ivy.legacy.domain.data.IvyCurrency but directive was com.ivy.wallet.domain.data.IvyCurrency in the data class file. I fixed it thus these changes appeared as-per correct package directive in places where it's used.
github-actions
bot
added
the
Stale
|
Thank you for your contribution to the Ivy Wallet project. We appreciate the time and effort you've invested in this pull request. As of Nov 5th, 2024, Ivy Wallet is no longer maintained. We are closing all open pull requests as part of the project shutdown. While we won't be merging this pull request, you're welcome to fork the repository and continue development independently under the terms of the GPL-3.0 License. For more information, please refer to the shutdown notice in our README file. We apologize for any inconvenience and thank you again for your interest in improving Ivy Wallet. |
Pull request (PR) checklist
Please check if your pull request fulfills the following requirements:
Screen recording of your changes (if applicable):
What's changed?
Describe with a few bullets what's new:
I've added below in FormatMoneyUseCase
Risk factors
What may go wrong if we merge your PR?
In what cases won't your code work?
Does this PR close any GitHub issues? (do not delete)
Troubleshooting GitHub Actions (CI) failures ❌
Pull request checks failing? Read our CI Troubleshooting guide.