MikeMcl / decimal.js

An arbitrary-precision Decimal type for JavaScript
http://mikemcl.github.io/decimal.js
MIT License
6.35k stars 480 forks source link

Suggestion: new method `toFixedNumber(...)` that returns a number #233

Closed AbdelrahmanHafez closed 6 months ago

AbdelrahmanHafez commented 6 months ago

I worked on many financial applications where the standard is to return all numbers with a maximum of 2 decimal places. What that means is that I almost always have to do things like:

Number(
    Decimal.sum(
        interestAmount,
        feesAmount
    ).sub(
        paidAmount,
        waivedAmount
    ).toFixed(2)
);

It would be so nice if we could get a new method, something along the lines of .toFixedNumber(...), that would return a number instead of a string. This would make our code much nicer, and I believe it's a common-enough use-case to justify implementing this feature. The could then would look like:

Decimal.sum(
    interestAmount,
    feesAmount
).sub(
    paidAmount,
    waivedAmount
).toFixedNumber(2)

I understand that Decimal.js tries to follow JS standards, so perhaps we could avoid the naming of toFixed altogether in favor of something more specific to Decimal.js. Happy to submit a PR if you agree to the idea.

AbdelrahmanHafez commented 6 months ago

Just found out about toDecimalPlaces which has an alias toDP after nearly a decade of frustration of having to unnecessarily add another level of indentation to my code.

AbdelrahmanHafez commented 6 months ago

I'm opting to leave this issue open for the time being. The toDecimalPlaces(...) method does alleviate the problem to an extent, as it spares us from adding an additional level of indentation. However, it also introduces a new challenge.

The method returns a Decimal object, which is logical given its functionality. Yet, this becomes problematic when I primarily use toDecimalPlaces(...) in the context of preparing data for JSON API responses. These responses are subjected to JSON.stringify(...), which, when encountering a non-primitive type like Decimal, defaults to invoking the .toString() method. Consequently, the value is transformed into a string before reaching the client.

At present, my alternatives are somewhat limited:

  1. Utilize Number(new Decimal(...).toFixed(2));, which feels overly cumbersome due to its nested structure.
  2. Employ new Decimal(...).toDP(2), which unfortunately results in a string being sent to the client.
  3. Opt for new Decimal(...).toDP(2).toNumber(), which, despite being a two-step process, seems like a more palatable option than the first.

A more streamlined solution, such as new Decimal(...).toDPN(2) (short for toDecimalPlacesNumber), would be highly beneficial for enhancing conciseness. However, I acknowledge that this suggestion might deviate from the library's established design principle of methods returning a Decimal object, which could potentially introduce breaking changes.

calebergh commented 6 months ago

@AbdelrahmanHafez - This idea, unfortunately, won't work. Javascript won't preserve decimal places in numbers - which is why the existing methods return strings. You can test this yourself: console.log(1.20) will output 1.2, console.log(1.00) will output 1. To get fixed decimal places, you must generate a string. It's just the way Javascript handles number.

While it would be great if we could get this, it's impossible. This issue should be closed.

AbdelrahmanHafez commented 6 months ago

@calebergh I think there is a misunderstanding. I am aware that 1.20 is the same as 1.2 in JavaScript, and that is perfectly fine.

What I'm proposing is an alias to decimal.toDP(2).toNumber() without any difference in the behavior, the comments above is me explaining why I need it, it's a very common use-case in my experience, and there is currently no concise way to achieve such a common functionality.

calebergh commented 6 months ago

Ah. I see. It's clarified in your later comments.

Seems to me a small utility function that does this for you would provide you with the output you want, and still be very simple to use...

const toDecimal = (num: Decimal): number => {
    return num.toDP(2).toNumber();
}

And just use toDecimal wherever it's needed.

It's my guess the requested feature won't get added, because it's pretty trivial to do. But I'm curious what the package author thinks :)

MikeMcl commented 6 months ago

Thanks for the suggestion @AbdelrahmanHafez. Although I can see that adding such a method could be useful for some users, there is a high bar to further additions to the API that is not quite met here.