elixirmoney / money

Elixir library for working with Money safer, easier, and fun... Is an interpretation of the Fowler's Money pattern in fun.prog.
https://hex.pm/packages/money/
MIT License
826 stars 139 forks source link

Adds support for :strip_insignificant_fractional_unit option for to_string #183

Closed darrenklein closed 2 years ago

darrenklein commented 2 years ago

Thank you very much for all of your hard work on this repo!

This PR adds an option to the to_string function that I felt people might find useful - it will strip the delimiter and fractional units from the result only if the fractional units are entirely insignificant zeros.

For example,

> Money.to_string(usd(500), strip_insignificant_fractional_unit: true)
"$5"

> Money.to_string(usd(550), strip_insignificant_fractional_unit: true)
"$5.50"

> Money.to_string(usd(505), strip_insignificant_fractional_unit: true)
"$5.05"

I hope you find this useful, I really appreciate the opportunity to make a contribution.

Nitrino commented 2 years ago

Hi @darrenklein It seems we already have the strip_insignificant_zeros parameter, isn't that what you need?

iex(5)> Money.to_string(Money.new(500, :USD), strip_insignificant_zeros: true)
"$5"
iex(8)> Money.to_string(Money.new(500, :USD), strip_insignificant_zeros: false)
"$5.00"

iex(9)> Money.to_string(Money.new(550, :USD), strip_insignificant_zeros: true)
"$5.5"
iex(10)> Money.to_string(Money.new(550, :USD), strip_insignificant_zeros: false)
"$5.50"
darrenklein commented 2 years ago

Thanks for writing, @Nitrino - it's true that the :strip_insignificant_zeros option is close to what I'm suggesting, but not quite the same. The option that I propose, when set to true, will trim the delimiter and fractional part only if the fractional part is entirely zeros - while this could be achieved with the :strip_insignificant_zeros option, you'd need to inspect the Money map before calling to_string in order to know how to set the option.

Consider a scenario where we have a list of Money maps to convert, where we only want to include the fractional part if it's relevant to the final value - so a list of maps with the amounts 500 and 550 (both :USD) would be processed into the list of strings ["$5", "$5.50"] -

> vals = [Money.new(500, :USD), Money.new(550, :USD)]
> Enum.map(vals, fn %{amount: amount} = money ->
  fractional_unit =
    amount
    |> Integer.to_string()
    |> String.slice(-2..-1)

  if fractional_unit == "00" do
    Money.to_string(money, strip_insignificant_zeros: true)
    # or you could use Money.to_string(money, fractional_unit: false)
  else
    Money.to_string(money)
  end
end)

["$5", "$5.50"]

and that's fine - but with the option I propose, this could be

> vals = [Money.new(500, :USD), Money.new(550, :USD)]
> Enum.map(vals, fn money ->
  Money.to_string(money, strip_insignificant_fractional_unit: true)
end)

["$5", "$5.50"]

What do you think? Does that seem useful?

Nitrino commented 2 years ago

@darrenklein Thanks for the explanation, now I understand. Yes, that seems like a useful option. Can you please resolve the conflicts and I will accept PR

darrenklein commented 2 years ago

@Nitrino Excellent, I'm glad you think this is useful, thank you so much for the opportunity to contribute! I've gone ahead and resolved those conflicts, this should be ready for merge now.

Nitrino commented 2 years ago

@darrenklein Thanks ❤️