Updated FormatMoneyUseCase #3657

shamim-emon · 2024-11-01T01:37:44Z

Pull request (PR) checklist

Please check if your pull request fulfills the following requirements:

I've read the Contribution Guidelines and my PR doesn't break the rules.
I've read and understand the Developer Guidelines.
I confirm that I've run the code locally and everything works as expected.
My PR includes only the necessary changes to fix the issue (i.e., no unnecessary files or lines of code are changed).
🎬 I've attached a screen recording of using the new code to the next paragraph (if applicable).

Screen recording of your changes (if applicable):

What's changed?

Describe with a few bullets what's new:
I've added below in FormatMoneyUseCase

Crypto formatting
KDocumentation

Risk factors

What may go wrong if we merge your PR?

N/A

In what cases won't your code work?

N/A

Does this PR close any GitHub issues? (do not delete)

Closes [FEATURE] Add KDoc And Crypto Formatting #3642

Troubleshooting GitHub Actions (CI) failures ❌

Pull request checks failing? Read our CI Troubleshooting guide.

ILIYANGERMANOV

Left some comments but "1,234.000000000000" isn't expected

ILIYANGERMANOV · 2024-11-01T18:50:02Z

shared/ui/core/src/main/java/com/ivy/ui/FormatMoneyUseCase.kt

+     * @return The formatted string representation of the value.
+     */
+    suspend fun format(value: Double, shortenAmount: Boolean, isCrypto: Boolean = false): String {
+        val result = if (isCrypto) {


You can directly return, no need for result var

ILIYANGERMANOV · 2024-11-01T18:51:34Z

shared/ui/core/src/main/java/com/ivy/ui/FormatMoneyUseCase.kt

+    private fun formatCrypto(value: Double): String {
+        val result = cryptoFormatter.format(value)
+        return when {
+            result.lastOrNull() == localDecimalSeparator().firstOrNull() -> {


I don't like this logic - can we just fix the formatter?

Removed the function, just relying on the suggested formatter now

ILIYANGERMANOV · 2024-11-01T18:52:33Z

shared/ui/core/src/main/java/com/ivy/ui/FormatMoneyUseCase.kt

@@ -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))


Can you try fixing the formatter by "#".repeat(9)

ILIYANGERMANOV · 2024-11-01T18:53:01Z

shared/ui/core/src/test/java/com/ivy/ui/FormatMoneyUseCaseTest.kt

+            shortenAmount = false,
+            isCrypto = true,
+            locale = Locale.ENGLISH,
+            expectedOutput = "123,456.000000000"


We don't expect such zeros

Updated the result as-per the suggested formatter expected result

ILIYANGERMANOV · 2024-11-02T23:05:01Z

shared/ui/core/src/main/java/com/ivy/ui/FormatMoneyUseCase.kt

@@ -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))


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)

ILIYANGERMANOV · 2024-11-02T23:05:50Z

shared/ui/core/src/test/java/com/ivy/ui/FormatMoneyUseCaseTest.kt

+            locale = Locale.ENGLISH,
+            expectedOutput = "123456"
+        ),
+        GERMAN_SHOW_DECIMAL_CRYPTO(


We need a tear case for "0.000345 BTC" for example

ILIYANGERMANOV · 2024-11-02T23:06:29Z

screen/edit-transaction/src/main/java/com/ivy/transaction/EditTransactionScreen.kt

@@ -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


What caused this package change?

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.

ILIYANGERMANOV · 2024-11-05T16:06:31Z

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.

shamim-emon requested a review from ILIYANGERMANOV as a code owner November 1, 2024 01:37

Updated FormatMoneyUseCase

0b8d70d

shamim-emon force-pushed the fix-issue-3642 branch from 9d56c95 to 0b8d70d Compare November 1, 2024 06:18

Empty-Commit

6900c68

ILIYANGERMANOV suggested changes Nov 1, 2024

View reviewed changes

ILIYANGERMANOV added the requested changes Changes are needed. Please, apply them label Nov 1, 2024

shamim-emon added 2 commits November 2, 2024 06:10

Updated FormatMoneyUseCase - reviews resolved

1db738f

Updated FormatMoneyUseCase -Empty-Commit

9cce740

shamim-emon requested a review from ILIYANGERMANOV November 2, 2024 00:33

ILIYANGERMANOV reviewed Nov 2, 2024

View reviewed changes

Updated FormatMoneyUseCase - reviews resolved

bd9f88a

shamim-emon requested a review from ILIYANGERMANOV November 3, 2024 02:25

github-actions bot added the Stale No activity, will be automatically closed soon. label Nov 5, 2024

ILIYANGERMANOV closed this Nov 5, 2024

shamim-emon deleted the fix-issue-3642 branch November 5, 2024 16:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated FormatMoneyUseCase #3657

Updated FormatMoneyUseCase #3657

shamim-emon commented Nov 1, 2024 •

edited

Loading

ILIYANGERMANOV left a comment

ILIYANGERMANOV Nov 1, 2024

shamim-emon Nov 2, 2024

ILIYANGERMANOV Nov 1, 2024

shamim-emon Nov 2, 2024

ILIYANGERMANOV Nov 1, 2024

shamim-emon Nov 2, 2024

ILIYANGERMANOV Nov 1, 2024

shamim-emon Nov 2, 2024

ILIYANGERMANOV Nov 2, 2024

shamim-emon Nov 3, 2024

ILIYANGERMANOV Nov 2, 2024

shamim-emon Nov 3, 2024

ILIYANGERMANOV Nov 2, 2024

shamim-emon Nov 3, 2024

ILIYANGERMANOV commented Nov 5, 2024

Updated FormatMoneyUseCase #3657

Updated FormatMoneyUseCase #3657

Conversation

shamim-emon commented Nov 1, 2024 • edited Loading

Pull request (PR) checklist

Screen recording of your changes (if applicable):

What's changed?

Risk factors

Does this PR close any GitHub issues? (do not delete)

Troubleshooting GitHub Actions (CI) failures ❌

ILIYANGERMANOV left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ILIYANGERMANOV commented Nov 5, 2024

shamim-emon commented Nov 1, 2024 •

edited

Loading