Added a number formatter to address issue #80 #86
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… user's locale
=============================== RECAP ============================================
1. Canceled localization of numbers done on the data layer in order to :
a. Localize only the text displayed to the user from the UI layer
b. Keep business logic language independent
------------ Tasks done ------------
- Updated FormattingEvaluator::evaluate to not localize the returned result
- Updated EventHandler to insert a "." instead of a localized decimal separator for CalculatorEvent.Decimal
2. Created the NumberFormatter class in the UI layer to hold number formatting logic based on the user's locale (ui/components/NumberFormatter.kt)
3. Formatted the numbers displayed by the calculator screen and the converter screen
4. Added a few UI ajustement to both the calculator screen and the converter screen
------------ Tasks done ------------
- Added to the converter screen a custom keypad that have a localized decimal separator and supports hexadecimal digits if base conversion is needed
- Updated the layout of the converter screen to acomodate the custom keypad
- Added ConverterViewModel to hold the user's input from the converter screen as a state
- Updated CalculatorViewModel::setExpression to place the displayText's cursor at the end after updating its value to make it ready to receive new inputs
- Added the user's locale to the calculator screen's keypad to make sure the decimal separator matches the user's locale
Bnyro
requested changes
Dec 26, 2025
Member
Bnyro
left a comment
Bnyro
left a comment
There was a problem hiding this comment.
Thanks for your interest in the app.
I don't want to attack you, but seeing this code, I just want to let you know that we do NOT accept any AI-generated code here. If this code was indeed AI-generated, I unfortunately can't merge these changes.
- The hexadecimal number input is not related to this PR (number format localization) and thus should be in a different PR
- Relying on regular expressions for the number formatting looks like a very bad idea and the "99 digits" assumption doesn't make any sense. If we want to do localization, we should use a library for this instead of re-inventing the wheel.
- The
ConverterViewModellogic should be moved to the already existingCalculatorViewModel
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Details in the commit message.