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

[11.x] Add precision to Number::currency() #54456

Merged
merged 1 commit into from
Feb 5, 2025
Merged

Conversation

benjibee
Copy link
Contributor

@benjibee benjibee commented Feb 3, 2025

This pull request adds support for specifying precision when formatting currency with the Number helper.

The most common use-case for this is showing larger prices (e.g. $299 or $1000) where the numbers after the decimal place aren't desired in certain contexts. Because different locales use different symbols for decimal places this can't be done properly after the fact.

@shaedrich
Copy link
Contributor

#54360 and #54163 may be interesting to you

@benjibee
Copy link
Contributor Author

benjibee commented Feb 3, 2025

#54360 and #54163 may be interesting to you

I hadn't seen these, thanks for pointing them out. The use-case and implementation here is a bit more limited in scope as I'm not trying to adjust the way anything is calculated or how the data is passed into the helper.

If there was a good workaround I would use it. But because of the way locales and NumberFormatter work, this really needs to be a parameter passed to the helper to work properly.

@shaedrich
Copy link
Contributor

I hadn't seen these, thanks for pointing them out.

You're welcome.

But because of the way locales and NumberFormatter work, this really needs to be a parameter passed to the helper to work properly.

However, precision is usually associated with number formatting, not so much with money. That's why we discussed a currency-based approach in #54163 as they can be three decimal places for some currencies and two for others, which you would overwrite with a static precision.

@crynobone crynobone changed the title Add precision to Number::currency() [11.x] Add precision to Number::currency() Feb 4, 2025
@taylorotwell taylorotwell merged commit 012ba06 into laravel:11.x Feb 5, 2025
40 checks passed
@donnysim
Copy link
Contributor

donnysim commented Feb 5, 2025

I think this is safer and less impactful as it's not assuming anything whether it's in cents or not and you have to provide the correct value for it instead of the previous suggestion where it would've made more sense to separate the function to avoid confusion because of the whole int vs float confusion.

@shaedrich
Copy link
Contributor

I disagree: "cents" might not have been the right naming, but we have discussed in the linked PR, that different currencies have different minor units (if at all). So this is not primarily a custom styling choice for the output but rather something currency-specific (similar to locale-specific formatting) that can't be achieved with this solution

@donnysim
Copy link
Contributor

donnysim commented Feb 5, 2025

Yeah, and this does not try to solve that situation. It just uses the same precision as Number::format which means the same thing no matter if you pass int or float. The linked PR introduced explicit inCents option, which changes the value meaning completely where precision could mean anything - cents precision or decimal precision, and you'd still be confused if you provide value in cents and have 3 cent precision but want to output currency in 2 decimal precision if used with current Number::currency if you don't divide the value explicitly.

From my point of view and the whole thought process and discussion previously, I don't see any way to add "cents" support to current Number::currency without making a huge confusing mess.

@shaedrich
Copy link
Contributor

It's only confusing if you insist on thinking in precision. These are to different concepts.

@donnysim
Copy link
Contributor

donnysim commented Feb 5, 2025

I mean, if you have a value 21234 that means 21.234 but you need to output it as $21.23, there is no understandable approach using the Number::currency even with the previous pull request, neither with this one, the difference is, this one doesn't try to solve it and leaves it to you to provide a float in that case instead of cents and then have to specify how many cents do you expect in your int. You would need to add as discussed previously $inCents parameter as int|false and in addition to that also add $precision, but then the whole thing crumbles the moment you provide a float and you're not sure whether the $inCents even does anything or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants