money_column currency option
elfassy opened this issue · 2 comments
For the next major release we should look into changing money_column to support a single currency. Rather than both currency
and currency_column
, we could have only currency
and make it accept something responding to call
or to_s
.
Example:
class Product
money_column :price, currency: ->(product) { product.currency }
money_column :price_usd, currency: 'USD'
end
I think this is easier to reason about and will help clean up the code
cc: @bdewater
I'm having a bit of deja vu 😄 we've had a similar discussion in the past: passing a symbol would refer to a method/attribute and a string would mean a hardcoded currency. We opted for the current solution with the currency_column
and currency
keyword arguments at the time.
I agree the money_column
should be nicer but I'm not convinced a lambda is worth the performance impact:
require "benchmark/ips"
def lambda
->(x) { x.zero? }
end
def method_send
0.send(:zero?)
end
def method_call
0.zero?
end
Benchmark.ips do |x|
x.report("lambda") { lambda.call(0) }
x.report("method send") { method_send }
x.report("method call") { method_call }
x.compare!
end
$ ruby -v test.rb
ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-darwin17]
Warming up --------------------------------------
lambda 114.692k i/100ms
method send 268.351k i/100ms
method call 296.777k i/100ms
Calculating -------------------------------------
lambda 1.598M (± 2.2%) i/s - 8.028M in 5.025326s
method send 7.275M (± 2.8%) i/s - 36.496M in 5.020949s
method call 9.247M (± 2.6%) i/s - 46.297M in 5.010301s
Comparison:
method call: 9246686.8 i/s
method send: 7274506.0 i/s - 1.27x slower
lambda: 1598367.5 i/s - 5.79x slower
another alternative would be hash specifically for the currency options
something like
class Product
money_column :price, currency: { column: 'currency', read_only: true }
money_column :price_usd, currency: 'USD'
end