#862 ✓resolved
Paul Sadauskas (Rando)

Float Type converts things that aren't floats to 0.0

Reported by Paul Sadauskas (Rando) | May 22nd, 2009 @ 03:37 PM | in 0.10.0

Because the value gets auto-cast, its impossible to validate the input was really a number, or actually "0.0"

class Test
  include DataMapper::Resource
  property :id, Serial
  property :value, Float
# => #<DataMapper::Property @model=Test @name=:value>
t = Test.new(:value => 3.14)
#=> #<Test @id=nil @value=3.14>
t = Test.new(:value => "Pi")
# => #<Test @id=nil @value=0.0>
# => true

Comments and changes to this ticket

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) May 22nd, 2009 @ 10:07 PM

    • Assigned user set to “Dan Kubb (dkubb)”
    • State changed from “unconfirmed” to “confirmed”

    I think the current typecast logic uses String#to_f on the incoming value, which is wrong due to how permissive Ruby is with it's #to_* methods.

    The fix for this is to spec the Property#typecast behavior for each of the primitives and included types, and make sure they are only coerced into the Ruby object if the value is a perfect match against a Regexp.

  • Paul Sadauskas (Rando)

    Paul Sadauskas (Rando) May 23rd, 2009 @ 12:41 AM

    I think this a perfect example of why we shouldn't be casting on assignment at all, except in very specific cases (hash of time values into a time, eg). The adapter should be the one deciding if it can handle a certain value. If the adapter cannot, then leave it up to a validation to catch it. A regex seems nice, but seems like it could slow down resource initialization a great deal.

    Just like how one of the most annoying gripes about mysql is its promiscuous tendency to shove values in however it can. http://sql-info.de/en/mysql/gotchas.html#1_13

  • Paul Sadauskas (Rando)

    Paul Sadauskas (Rando) May 23rd, 2009 @ 12:51 AM

    Same problem with Integer types, see #860

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) May 23rd, 2009 @ 11:45 AM

    Resource initialization happens in two main instances: when loading from an adapter, and loading based on user input.

    This change should not affect how adapter loads happen, since those bypass most of the coercions, because we assume the values are coming back are the proper types and set the @ivar values directly. This all ties into the per-adapter typecast (or coercion, whatever you want to call it), which is (or should be) the subject of another ticket so I won't talk more about it here.

    When loading based on user input, it is extremely useful to be able to coerce values to the right Ruby object. I use it in every app I build, and so do 99% of the apps that rely on DM. (ok I'm pulling that 99% figure out of thin air, but the dozen or so people I've mentioned removing this feature to has said they rely on it heavily, as do I) We've gone around and around this, but the feature it not going to be removed from dm-core.

    Furthermore, the specific strategy I mentioned above (using a Regexp to validate the data prior to coercion) is only for a very specific case: when the value has been provided in a String form for a Property that is not a String. In a stand-alone app, where you hard code to the values, get them in the right type from a trusted source or coerce them in the caller yourself, this will be skipped.. so the "happy path" will not be affected.

    This is only in the very specific case where you provide a String value to a mutator for a non-String property. This means that when Resource#attributes is passed a Hash of String values, it'll do the right thing and coerce them to the expected types only if they exactly match expectations.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) June 16th, 2009 @ 03:20 AM

    • Milestone set to 0.10.0
  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) June 16th, 2009 @ 10:41 PM

    • State changed from “confirmed” to “accepted”
  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) June 22nd, 2009 @ 10:03 PM

    • State changed from “accepted” to “resolved”

    (from [8eb8e44cc15c8e32b21f12c96650452a886f1678]) Updated numeric typecasting to only work on numbers

    • Updated specs for Integer, Float and BigDecimal
    • Refactored typecasting for each primitive into separate methods

    [#862 state:resolved] http://github.com/datamapper/dm-core/commit/8eb8e44cc15c8e32b21f12c...

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Referenced by