Rails: add support for :decimal field instead of cents/integers

[gamov]

Trivial to implement but would be nice to have it built-in

[semmons99]

You should be able to use #to_f to do this already.

[weppos]

I agree with gamov.

No longer than yesterday I had to implement a new routine in one Rails app to deal with money. Indeed, users can't play with cents. I can't ask to an user "Enter here your order price in cents".

While I was working on the feature, I realized that Money could probably be improved on this side. I'd like to propose a few additional methods:

Money.parse([String])

Equivalent to String.to_s.to_money or viceversa.
Input is always handled as string.

Ideally, I would be able to call

Money.parse("10")
Money.parse("10 USD")

Money.new_from_decimal([Fixnum/Float], ...) (or ala Objective-C new_with_decimal)

Equivalent to Money.new, but accepts a decimal value instead of cents.
Accepts a currency and a bank, as Money.new does.
It automatically converts the input value to cents,
according to the value of subunit for given currency.

Money#decimal
Money#decimal=

Gets/Sets the value in Fixnum/Float.

I'm sure these methods will significantly improve the library.
I can work on them once we agree on the implementation. :)

[semmons99]

What about just enhancing #initialize to accept any Object and attempting to create a new Money object based on the Class? Something like the code below.

class Money
  def initialize(obj,
                 currency = Money.default_currency,
                 bank     = Money.default_bank)
    case obj
    when Fixnum
      # do what we do now
    when String
      # use String#to_money
    when Numeric
      # use Numeric#to_money
    else
      # raise an error
    end
  end
end

[weppos]

Honestly, I would consider to keep them separate.
I don't really like superclever methods because it's really easy to introduce additional and hard-to-debug bugs.

For instance, it would be hard to understand if Money.new(10) should be 10 cents or 10 dollars.

I believe using separate method provides additional security and readability, in this case.

[semmons99]

Okay, I can agree with that. How about something like this then?

# Money#from_string
def self.from_string(s,
                     currency = Money.default_currency,
                     bank = Money.default_bank
  #...
end

# Money#from_float
def self.from_float(n,
                    currency = Money.default_currency,
                    bank = Money.default_bank
  #...
end

They're unambiguous, and we can easily add more methods in the future. In this case I wouldn't use String#to_money and use #to_f on the provided string, not have to jump through all the hoops String#to_money attempts.

[weppos]

I agree.

Would this implementation also include Money.parse?

[semmons99]

I'd avoid #parse and just use #from_* that way there's a consistent way to do things when you're not passing cents to #new.

[semmons99]

do you want to write these methods weppos?

[weppos]

Sure, I can work on them. I'll try to get the patch within tomorrow.

[gamov]

I agree with weppos that it must be clear when Money will deal with :decimal instead of cents.
However, I think it would be great if the new version of Money support the composed_of method of Rails and transparently detects if data from the AR backend is in :decimal/integer (checking if it's a BigDecimal type).
[I'm new to Ruby and Rails... please bare with me if I don't make much sense :o)]
If I understand correctly, we can play around with :constructor/:converter parameters to 'interface' between the DB data and the Money class and make the dec->cents conversion there. Am I right?

Why not #from_dec? It seems you consider the evil float equal to the righteous Bigdecimal.

[weppos]

We always try to avoid coupling the Money library with Rails. Once these methods will be in place, you'll be able to accomplish the task defining your own composed_of configuration.

[semmons99]

We could add a #from_bigdecimal method as well.

[semmons99]

I can implement these if you don't have time, weppos.

[weppos]

I'm quite busy right now, but I'd like to work on it to break from my current daily activity.
Give me a few hours. :)

[semmons99]

sounds good :)

[gamov]

hey guys, one more newbie question: I wanted to had the blue 'enhancement' label to this issue, I couldn't figure how. Do you need some kind of admin rights?

[semmons99]

You have to be a contributor to add labels, close issues, etc.

[gamov]

Cool. thanks for your help!

[weppos]

Now that I have a few minutes to stop and relax, I'm reviewing all the comments to this issue to organize the changes.

I'd like to summarize all the activities for a final feedback. Honestly, there's also a small piece of these changes that doesn't please me very much. I'm going to explain why.

First, the goal of this ticket: add support for getting/setting/creating Money objects from a decimal value. We can define decimal value the "human readable" amount of money, which Money internally manages as cents.

# Current Money implementation
# You can't supply $10 but you need to convert the amount to cents
Money.new(1000, "USD") # 1000 cents == 10 $

The idea is to have something like

Money.__METHOD__(10, "USD")
# $10 are converted into 1000
# then Money.new(1000) is called.

Here's my first question. I'm not a native English/American speaker so forgive me if this is a stupid question. We initially agreed on using decimal but, to me, the word decimal doesn't really sound correct. It is too tied to a data tipe, even if no Decimal class exists in Ruby. I would use a more abstract word, something like amount or value.

Money.new(1000, "USD")
Money.from_amount(10, "USD")

This word will be consistently use across the entire library, so that we can implement

m = Money.new(1000, "USD")
m.amount # => 10

m.amount = 12
m.cents  # => 1200

Needless to say, the amount <=> cents conversion will be aware of the Currenty#subunit property.

Second question is: do we really need a list of

Money.from_bigdecimal
Money.from_integer
Money.from_string
Money.from_whatever

In my opinion, we should abstract the money implementation from a specific data type and the ticket #27 is an excellent demonstration.

I believe Money should provide methods to create money from "concepts", rather than from "types".

What does it means in practice?

First, we can provide a reasonable set of allowed types when defining a method API. For instance, Money.new accepts cents and we can intelligently try to guess whether cents is a Float/Bigdecimal/Fixnum and coherce it. On this side, I would recommend to avoid super-clever methods. In fact, a Money.new which accepts also Strings will probably cause much more headache in debugging apps than help.

Then, we can add more "meaningful" methods to extend Money usage always trying to work with concepts rather than types (Ruby is not really a strongly typed language).

---

There's an small exception to my feedback. We can think about adding more .from_type methods with the purpose to simplify the core extensions provided in the core_extensions.rb file. In this case, we can create

# for the purpose of this example I separate Numeric
# into smaller classes
Money.from_fixnum(value, ...)
Money.from_float(value, ...)
Money.from_bigdecimal(value, ...)

class Fixnum
  def to_money(...)
    Money.from_fixnum(self, ...)
  end
end

class Float
  def to_money(...)
    Money.from_float(self, ...)
  end
end

class BigDecimal
  def to_money(...)
    Money.from_bigdecimal(self, ...)
  end
end

String deserves a special mention here because a String is a very data-reach money format. In fact, there are two types of string representations:

# with currency
"10 $"
"10 USD"
# without currency
"10"

This is the reason why we can try to provide two methods:

# A type-focused method, as for BigDecimal, Fixnum...
# which accepts only digit representations
# "1.0", "1000" and handle them as cents
Money.from_string

# A concept-focused method which implements a more advanced
# (and slow) detection mechanism
Money.parse(...)

The idea behind this change is to clean up the core_extensions by moving all the logic into the Money class.

---

To summarize:

1. Do you agree with the idea of using a less type-coupled term? Which one? Assuming to use amount, I would add

   # I would use the new_from prefix to prevent confusion with
   # type-focused methods such as from_numeric which always
   # works with cents as expected.
   Money.new_from_amount(numeric, ...)
   
   Money#amount
   Money#amount=(numeric)
   

2. Do you agree with the idea to cleanup core extensions and add type-coupled initialization methods? I would add

   Money.from_bigdecimal
   Money.from_numeric  # (or from_fixnum, from_float)
   Money.from_string
   Money.parse
   

and update the core extensions accordingly.

We can also decide to move the section 2 to an other ticket.

---

Sorry for the very long comment.

[semmons99]

I like the ideas, but the word amount is confusing. Since we're already using cents I would suggest using the word dollars as it is more universally understandable, and corresponds well to cents.

I'd also like to see a separate ticket (and set of patches) submitted for the core extensions cleanup.

[weppos]

I don't like very much the word dollars because it reminds the Dollar currency name, however at the moment I don't have a better name so I used it as suggested.

See http://github.com/RubyMoney/money/compare/master...issue_25

Any feedback before the merge?

There are a couple of FIXME. I'm planning to remove them once the core extension refactoring is completed and we addressed a couple of other related issues I posted today.

[semmons99]

Looks good, one minor thing, in the specs, can you use #new instead of .new in the descriptions?

[weppos]

Sure!
I used # to identify instance methods (such as #dollars) and . to identify class methods (such as .new_from_dollars).

[weppos]

Add support for creating objects with the main monetary unit instead of cents (closed by 1172245)

[gamov]

• since we already have #cents, what about #units (instead of dollars/amount)?

[weppos]

It please me more than #dollars. Let's see semmons99's feedback.

[semmons99]

Units is ambiguous, it can refer to either dollars and cents. I understands that dollars is specific to a handful of countries (approximately 36 countries use the dollar to refer to their whole unit of money), but so is cents (approximately 42 countries). I'd rather use a term that a majority of people understand and can reason about, than something more ambiguous that everyone would be forced to look at documentation for.

[weppos]

Ok, let's leave #dollars in place.

[semmons99]

If dollars/cents is an issue, we could always add two new keys to the Currency table for :whole_units and :fractional_units and then use #method_missing so that users could call their natural language currency units (which will just call #dollars and #cents. It's not something I think is necessary, but could be added if you think it's needed.

[gamov]

Yes, I think it's nicer to have those two additional methods (whole_units/fract_units) semmons99.
Note that when I evaluated other money gems, I also saw the use of dollars/cents method names even for other currencies.

[semmons99]

If we go this direction, I would actually add :subunit and :unit to Currency::TABLE and the use #method_missing to call #dollars and #cents.

[weppos]

At this point, I suggest to go ahead with the current implementation (cents and dollars) and keep things as simple as possible. We can always try to implement different solutions in the future, based to real-world usages and feedback.

[semmons99]

sounds good