Create format_number filter #999
Conversation
tkindy
left a comment
There was a problem hiding this comment.
Mostly just a couple naming things, but generally LGTM
src/main/java/com/hubspot/jinjava/lib/filter/NumberFormatFilter.java
Outdated
Show resolved
Hide resolved
| private String formatNumber( | ||
| Locale locale, | ||
| BigDecimal number, | ||
| int noOfDecimalPlacesInInput, |
There was a problem hiding this comment.
Since this parameter's value comes from number, I think we can get it inside this method instead of passing it as a separate parameter.
There was a problem hiding this comment.
Makes sense! In the main method, I was using this variable as a fallback to the max decimal precision variable if it wasn't passed in, but I ended up switching the max decimal precision variable to an Optional so it no longer needs to be a parameter for this method
| ) { | ||
| NumberFormat numberFormat = NumberFormat.getNumberInstance(locale); | ||
|
|
||
| numberFormat.setMinimumFractionDigits(noOfDecimalPlacesInInput); |
There was a problem hiding this comment.
Do you need to set this, actually? I'd think the default NumberFormat would try to show the entire passed-in value by default, which would mean it would use all decimal digits (unless it was over the maximum).
There was a problem hiding this comment.
I double checked with the unit tests I wrote and I think you are correct. This doesn't need to be set since by default - it'll just output all the decimal digits unless a maximum less than the number of decimal digits was set
| @JinjavaSnippet(code = "{{ number|format_number(\"en-US\", 3) }}") | ||
| } | ||
| ) | ||
| public class NumberFormatFilter implements Filter { |
There was a problem hiding this comment.
I think FormatNumberFilter would align with how filters are named relative to their representation in Jinjava, no?
| Md5Filter.class, | ||
| MinusTimeFilter.class, | ||
| MultiplyFilter.class, | ||
| FormatNumberFilter.class, |
There was a problem hiding this comment.
This should go higher up to maintain the alphabetic sort now that the name has changed.
Since locales render numbers differently, there needs to be a way to format numbers based on the locale. Currently, the only way is to use the
format_currencyfilter and get rid of the currency symbol.This PR creates a
format_numberfilter that will do this formatting natively.format_number(locale, decimalPrecisionNumber)wherelocaleis String of an ISO language code andmaxDecimalPrecisionis a number that determines the number of decimal digits of the formatted input value.Examples:
{{ 1000|format_number('en-US') }}==> 1,000{{ 1000.333|format_number('en-US') }}==> 1,000.333{{ 1000.333|format_number('en-US', 2) }}==> 1,000.33{{ 1000|format_number('fr') }}==> 1 000{{ 1000.333|format_number('fr') }}==> 1 000,333{{ 1000.333|format_number('fr', 2) }}==> 1 000,33{{ 1000|format_number('es') }}==> 1.000{{ 1000.333|format_number('es') }}==> 1.000,333{{ 1000.333|format_number('es', 2) }}==> 1.000,33