patch / cldr-number-pm5

Localized number formatters using the Unicode CLDR
https://metacpan.org/pod/CLDR::Number
Other
8 stars 3 forks source link

Using Locale::CLDR corrupts CLDR::Number #52

Open MarcelVersteeg opened 2 years ago

MarcelVersteeg commented 2 years ago

In a project I am using CLDR::Number for quite some time to format numbers in the right locale.

Now I want to use Locale::CLDR to get country names in the correct language. However, as soon as I use Locale::CLDR, formatting an integer number via CLDR::Number fails with the message: Can't locate object method "ffround" via package "Math::BigInt" at <path_to>/perllib/CLDR/Number/Role/Format.pm line 260

I can easily reproduce this using the following script:

#!/usr/bin/perl

use strict;

use CLDR::Number;
use Locale::CLDR;

my $cldr = CLDR::Number->new(locale => 'en');
my $formatter = $cldr->decimal_formatter(minimum_fraction_digits => 2, maximum_fraction_digits => 2);
print 'Success: ', $formatter->format(15.23), "\n";
print 'Fail: ', $formatter->format(42.0), "\n";

Here, the formatting of the number 42 will fail with the indicated message. As soon as I remove the line use Locale::CLDR, the formatting works as expected.

Do you know why using Locale::CLDR causes CLDR::Number to break? I know that the latter is a somewhat older module, but I do not want to let go of it. If there is a more up-to-date module with a similar interface as CLDR::Number, then I will definitely check it out.

patch commented 2 years ago

Hi Marcel, thanks for reporting this. I’m not sure why Locale::CLDR causes CLDR::Number to break. I do wonder why ffround is associated with Math::BigInt in your error message, as it’s actually part of Math::BigFloat which I’m using. I’m not able to help troubleshoot at this time but would happily accept any fixes. I’m not familiar with any modern CLDR-related modules in CPAN, although I can show you how to easily accomplish this in JavaScript in case your project uses frontend code.

MarcelVersteeg commented 2 years ago

I will try to find out the actual cause of this (will also contact the developer of Locale::CLDR). I will let you know the outcome and if I am able to make a fix for it.

MarcelVersteeg commented 2 years ago

I have been debugging my test script, first without the use Locale::CLDR (which succeeds) and then with the use Locale::CLDR (which fails). I see some strange things, that I can't quite explain, but there is some difference.

Without use Locale::CLDR On line 260 of the file CLDR/Number/Role/Format.pm, you call Math::BigFloat->($num). As $num is the integer value 42, Math::BigFloat creates a Math::BigInt and stores that one internally. The call to ffround on this new object will actually result in a call to function Math::BigFloat::bfround (note the difference in the first character of the function name!) and this call will actually succeed. The module Math::BigFloat also has a module variable called $downgrade. When Locale::CLDR is not loaded, this variable has the value undef.

With use Locale::CLDR On line 260 of the file CLDR/Number/Role/Format.pm, you call Math::BigFloat->($num). As $num is the integer value 42, Math::BigFloat creates a Math::BigInt and stores that one internally. The call to ffround on this new object in this case does not result in a call to function Math::BigFloat::bfround (note the difference in the first character of the function name!), but indeed tries to call ffround on the Math::BigInt that was stored internally and that function does not exist. When the module Locale::CLDR is loaded, the module Math::BigFloat variable $downgrade has the value Math::BigInt. This probably makes the difference in handling the ffround.

Now examining the code of Math::BigFloat further, there is only one location where the modules variable $downgrade is set, and that is in its import function when one of the arguments is 'downgrade' (which does not seem to be documented on cpan BTW). The fact that $downgrade is set when using Locale::CDLR is because the module Locale::CLDR::NumberFormatter uses bignum.

My proposal would be to change the following code in the function CLDR::Number::Role::Format::_format_number (lines 258 to 264)

        else {
            # round half to even
            $rounded = Math::BigFloat->new($num)->ffround(
                -$self->maximum_fraction_digits,
                'even'
            )->babs->bstr;
        }

to

        else {
            # round half to even
            my $mbf = Math::BigFloat->new($num);
            if (ref $mbf eq 'Math::BigInt')     # Needed in case another module uses Math::BigFloat with the 'downgrade' option
            {
                $rounded = $mbf->bfround(
                    -$self->maximum_fraction_digits,
                    'even'
                );
            }
            else
            {
                $rounded = $mbf->ffround(
                    -$self->maximum_fraction_digits,
                    'even'
                );
            }
            $rounded = $rounded->babs->bstr;
        }

Would it be possible to make this change in the repository and create a new release of CLDR::Number in metacpan? Of course I can locally patch it, but that would not make it available to other users of this module.