Coder-Spirit / php-bignumbers

A robust library to handle immutable big numbers inside PHP applications
MIT License
131 stars 29 forks source link

Decimal formatter #39

Closed shabbyrobe closed 7 years ago

shabbyrobe commented 9 years ago

I ran into an issue where I started using Decimal objects but my views were using a variant of number_format, so I've written a simple Formatter class for Decimal which I figured I should offer back to the project. I originally made it a method directly on Decimal, but it got ugly pretty fast and then I remembered that there are countries in the world that aren't Australia!

I tried to find a flexible middle-ground between the clumsiness of number_format and the incredible completeness of NumberFormatter (which would be way too much work to replicate). Hopefully you find this a useful addition.

// basic usage:
$f = new \Litipk\BigNumbers\Formatter();
echo $f->format("1234567.89");
echo $f->format(\Litipk\BigNumbers\Decimal::create("1234567.89"));
// "1,234,567.89"

// customisable:
$f = new \Litipk\BigNumbers\Formatter([
    'thousandsMark'=>"'",
    'decimalMark'=>',',
    'padDecimal'=>false,
    'tpl'=>'YEAH! {num}{sign} !!!',
    'sign'=>' -',
    'scale'=>5,
]);
echo $f->format(\Litipk\BigNumbers\Decimal::create("-1234567.89"));
// "YEAH! 1'234'567,89 - !!!"

I added a simple registry so you can store formats as you please for easy access:

Formatter::register('aud', ['scale'=>2, 'tpl'=>'AUD ${sign}{num}']);
Formatter::formatAs('aud', '1.234');
Formatter::formatAs('aud', \Litipk\BigNumbers\Decimal::create('1.234'));
// "AUD $1.23"

There's some syntactic sugar too, i.e. you can also call Decimal->format($options) and Decimal->formatAs($registered), but all it does is delegate to the formatter.

shabbyrobe commented 9 years ago

Don't really know why scrutinizer is failing - it just says "Task Failed" and provides no details. Am I doing something wrong?

castarco commented 9 years ago

I'll take a look this sunday , surely it's a minor defect given your previous pull requests.

The idea is very interesting, but I want to check te static methods embedded in Formatter class.

I'm not against having a static reference to a singleton, but I want to be sure that it's possible to directly work with instances in order to make easier unit testing and other tasks where having global state is usually harmful.

shabbyrobe commented 9 years ago

I agree entirely with the dangers inherent in global state, but it can be convenient in certain situations when used with some discipline. I think it should always be a supplement to an instance-based approach rather than a replacement for it, and my examples above titled // basic usage and // customisable show that working with the instances directly is definitely a first-class citizen and the static stuff merely syntactic sugar.

The convenience is worthwhile though, at least in my eyes. The intention was to offer something halfway between the instance method (new Formatter(...))->format() and the PHP intl extension's new \NumberFormatter($locale, $style): the former can potentially be too verbose and repetitive if you have only a few specific formats application-wide (like my present use-case), the latter is way way way outside the scope of what we can reasonably provide (awesome as it is) ;)

There's really not that much separating these two methods conceptually:

// PHP's intl:
$formatter = new \NumberFormatter("en-US", \NumberFormatter::CURRENCY);

// BigNumbers:
$formatter = \Litipk\BigNumbers\Formatter::registered("USD");

Both return a formatter which is ready to go based on a statically configured dataset. The difference is that in the case of intl the data is provided by the extension, but in the case of the Bignumber formatter, it's provided by your own calls to register(...). (n.b. the Formatter::registered() method in the above example is missing from the pull request at the moment)

I'm not married to the static stuff though. I don't mind at all if you'd prefer it wasn't in there and will gladly remove it if it raises the chances that an instance-only Formatter is included, this is just a bit of explanation about why I put it in there.

segy commented 8 years ago

hello. this is quite an interesting feature. do you plan to merge it in the near future? thanks

castarco commented 8 years ago

Hello @shabbyrobe . After all this time, I've been thinking and I definitely want to merge your pull request. The are only two points to be "fixed":

Thank you for your effort, I apologize for the excessive waiting time.