Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in AbstractLanguage for decimalNumbers with zero at the end #133

Closed
PattaBov opened this issue Feb 16, 2024 · 8 comments
Closed

Bug in AbstractLanguage for decimalNumbers with zero at the end #133

PattaBov opened this issue Feb 16, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@PattaBov
Copy link

PattaBov commented Feb 16, 2024

Thank you for your work, this library is a huge help as we convert numbers to text in several languages for our project.

I need to point out a bug though.

When we want to convert some value with two decimals finishing with zero, like "12.50", we expect a string value of "twelve point fifty" but we actually get "twelve point zero fifty".

It comes from the decimalToCardinal function in AbstractLanguage.js:

// Loop through array (from the end) adding leading zeros to output array
chars.forEach(char => {
    if (char === '0') {
        words.push(this.zero);
    } else {
        allZero = false;
    }
});

I think it misses a break after

allZero = false;

Other small things, our german colleagues told us that komma in german takes a capital K so "12.50" => "zwölf Komma fünfzig"

Thank you for reading

@forzagreen forzagreen added the bug Something isn't working label Feb 16, 2024
@forzagreen
Copy link
Owner

Hi @PattaBov ,
Thanks for reporting.
Could you please provide the version you are using?
I haven't encountered these issues with the latest version (1.19.0):

> n2words(12.50)
'twelve point five'
> n2words(12.50, {lang: "de"})
'zwölf komma fünf'

In regards to representing the result "12.50" as "twelve point five", I believe we could offer an option to display it as "twelve point fifty" instead.

@TylerVigario
Copy link
Collaborator

TylerVigario commented Feb 17, 2024

I think it misses a break after

allZero = false;

@PattaBov Good catch! I'll get this updated ASAP

EDIT: Fixed in #134. The release will follow shortly since this is quite the find.

EDIT 2: Released in v1.19.1

@PattaBov
Copy link
Author

Hi, thanks for the update. We were using the version 1.17.0.
I'm gonna update the package and test it.

@PattaBov
Copy link
Author

That seems perfect to me, thank you for your quick reaction!

@PattaBov PattaBov reopened this Feb 21, 2024
@PattaBov
Copy link
Author

PattaBov commented Feb 21, 2024

Other small things, our german colleagues told us that komma in german takes a capital K so "12.50" => "zwölf Komma fünfzig"

My testing team just pointed me out that the output still is "komma" instead of "Komma".
Also my vietnamese colleague told me :

in VN we use "phẩy" instead of "Phẩy"

I can make a local workaround in our project, no problem, but could you include these changes in your next release ?

Thank you

@TylerVigario
Copy link
Collaborator

TylerVigario commented Feb 22, 2024

@PattaBov I only manage the project structure and keep it up-to-date with the latest JavaScript standards. Maybe @forzagreen can chime in with insight into languages, regions, and words.

However, I have made each language as flexible as possible. This means you can specify a different seperatorWord. Please take a look below for an example.

import n2words from 'n2words/i18n/de.js'

n2words(100, {
  seperatorWord: 'Komma'
})

@forzagreen
Copy link
Owner

Thank you, @TylerVigario , for the resolution.
@PattaBov , I suggest closing this issue and opening a new one.

@PattaBov
Copy link
Author

Thank you,
Tyler's suggestion is actually enough. I'm sorry I didn't think about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants