Skip to content
This repository

Rates.rb, Rails and Flt::DecNum #9

Closed
ReidCarlberg opened this Issue · 7 comments

5 participants

ReidCarlberg Bill Kranec bitterloa Thadd Selden Conrad Twizzle
ReidCarlberg

Hi Bill,

Working through your gem and it looks pretty sweet. Ran into some entertaining error that I had no idea how to solve so I took a deeper dive.

The error occurs with Ruby 1.9.3p0 and Rails 3.2.3.
When you attempt to something simple like

rate = Rate.new(0.04, :effective, :duration => 360)
amortization = Amortization.new(100000, rate)

You get an error

wrong argument type Flt::DecNum (expected scalar Numeric)

Investigating, this is the stack trace

lib/finance/rates.rb:161:in **'
lib/finance/rates.rb:161:in
to_nominal'
lib/finance/rates.rb:75:in effective='
lib/finance/rates.rb:101:in
initialize'
app/controllers/loans_controller.rb:49:in new'
app/controllers/loans_controller.rb:49:in
create'

If I hop over to Rates RB and update the @periods initializer from

def compounds=(input)
  @periods = case input
             when :annually     then Flt::DecNum 1
             when :continuously then Flt::DecNum.infinity
             when :daily        then Flt::DecNum 365
             when :monthly      then Flt::DecNum 12
             when :quarterly    then Flt::DecNum 4
             when :semiannually then Flt::DecNum 2
             when Numeric       then Flt::DecNum input.to_s
             else raise ArgumentError
             end
end

and update the "monthly" setting to

             when :monthly      then '12.0'.to_d

It seems to work.

Of the top of your head, do you know what I lose if I update all those Flt:DecNum initializers to something simpler?

Thanks & thanks for the great work on the gem.

Reid

Bill Kranec
Owner

Hi Reid,

To answer your question: no, I believe the result is equivalent using .to_d. I think I wrote this case statement before I got tired of initializing DecNum everywhere.

That said, I have a couple of follow-up questions:

  1. I wasn't able to replicate the error you got on ruby-1.9.3p0. Are you running the code from inside the rails console?
  2. The error that you are getting seems to imply that a DecNum is being passed as the compounding argument. I haven't explicitly accounted for this in my case statement, so I would be curious to see if adding the line when Flt::DecNum then input also fixes your problem.
  3. Again, I'm confounded by the different behavior under Rails (there are several similar issues on GitHub). If you have any suggestions for simple ways to test the functionality of the gem within the Rails environment, I'd love to hear it.

Thanks again for your feedback. It's always great to hear from people using the library.

Bill

ReidCarlberg

Ok thanks. I'm the newbiest of all rails newbies, so I have no idea what's happening. I ran the ruby tests outside of rails and it worked. What it looked like to me was an exception thrown when you try to call ".to_d" on an Flt::DecNum. I wonder if there's something in rails that overrides the standard to_d in a way that cases this.

bitterloa

hello, i am getting this exact same error using Rails 3.2.6 / Ruby 1.9.3p194. thanks so much for posting about this. is there anyway to resolve so that i may use the Finance gem?

and to the author (Bill), how could adding the "when Flt::DecNum then input" resolve?

thanks!

bitterloa

actually i tried adding the line "when Flt::DecNum then input" and this didn't resolve, but doing the '12.0'.to_d suggested by ReidCarlberg seemed to work. i'll do more testing. please do let me know any suggestions on how to properly use this with Rails. Thanks!

Thadd Selden thadd referenced this issue
Closed

Fix for #9 #16

Thadd Selden

Digging a little further, I think the core issue is that the core BigDecimal's to_d is beating out Finance's to_d on Numeric.

ruby-1.9.3-p194 :003 > 1.method(:to_d).source_location
NameError: undefined method `to_d' for class `Fixnum'
    from (irb):3:in `method'
    from (irb):3
    from ~/.rvm/rubies/ruby-1.9.3-p194/bin/irb:16:in `<main>'
ruby-1.9.3-p194 :004 > require 'bigdecimal/util'
 => true 
ruby-1.9.3-p194 :005 > 1.method(:to_d).source_location
 => ["~/.rvm/rubies/ruby-1.9.3-p194/lib/ruby/1.9.1/bigdecimal/util.rb", 13] 

bigdecimal/util is included by ActiveRecord:

activerecord-3.2.8/lib/active_record/connection_adapters/abstract/schema_definitions.rb:require 'bigdecimal/util'
activerecord-3.2.8/lib/active_record/connection_adapters/abstract_adapter.rb:require 'bigdecimal/util'

I tried adding this in an initializer:

module ToDConverter
  def to_d
    if self.instance_of? DecNum
      self
    else
      DecNum self.to_s
    end
  end
end

class Fixnum
  include ToDConverter
end

class Float
  include ToDConverter
end

But it only works for Fixnums, not for Floats too:

ruby-1.9.3-p194 :003 > 1.2.method(:to_d).source_location
 => ["~/.rvm/rubies/ruby-1.9.3-p194/lib/ruby/1.9.1/bigdecimal/util.rb", 30] 
ruby-1.9.3-p194 :004 > 1.method(:to_d).source_location
 => ["~/.../<my_rails_project>/config/initializers/fixnum_to_d_method_for_finance.rb", 2] 
Bill Kranec
Owner

My intent in defining the #to_d method was to integrate nicely with existing Ruby methods and define as few new methods on existing classes as possible. At least in this case, this now appears to be more trouble than it is worth.

I see two potential ways out:
1. Rename #to_d to something like #to_decimal. My guess is that there aren't many external functions that rely on the #to_d method as implemented in Finance, so this shouldn't break too much existing code.
2. We could drop the flt library and adopt BigDecimals internally. There are some drawbacks to this, documented at the bottom of the flt page, but nothing catastrophic. This could also make it easier to use Ruby's Newton's Method solver, which currently only accepts BigDecimals, and we have to convert back and forth.

I'm open to other suggestions as well if anyone has an opinion one way or the other.

Conrad Twizzle

Does this work with rails yet? It would be awesome if I could help out in some way.

Bill Kranec wkranec closed this issue from a commit
Thadd Selden thadd [Fixes #9]: Explicitly initializes the places where Flt::DecNum is ex…
…pected rather than relying on to_d
be13a1a
Bill Kranec wkranec closed this in be13a1a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.