#462 ✓resolved
Ana Nelson

BigDecimal without scale, precision doesn't validate as numeric

Reported by Ana Nelson | July 16th, 2008 @ 02:48 PM

  1. FIXME: if precision and scale are not specified, can we assume that it is an integer?

Well, you might also assume that someone has forgotten to specify :scale. :-)

The validation error message when you have a decimal point which doesn't correspond to scale/precision settings is rather cryptic:

{:bad_value=>["Bad value must be a number"]}

Especially if you have forgotten to specify scale and precision and scale has defauled to 0.

Suggestion attached.

Comments and changes to this ticket

  • Ana Nelson

    Ana Nelson July 16th, 2008 @ 02:50 PM

    and here's an example...

    oh...

    if you just do

    puts e.errors.inspect

    then you don't see the error message but if you do

    puts e.valid?

    puts e.errors.inspect

    then you do.

  • Mayo Jordanov

    Mayo Jordanov July 17th, 2008 @ 02:43 AM

    I don't think you can assume integer and/or force specifying scale and precision because floating point values don't really use scale and precision. (well, there is, but it's platform dependent. I guess precision = 15, scale = 14 would be reasonable)

    The code (http://github.com/sam/dm-more/tr...) should either break Float out and deal with it separately , or assume some safe defaults for scale and precision just for the sake of validating.

    Although, looking at the NumericValidator, it's not quite clear to me why precision and scale need to be specified ... line 31 (http://github.com/sam/dm-more/tr...) has a failover with the regexp on line 42 (http://github.com/sam/dm-more/tr...), which will return true for Floats. I'll investigate some more tomorrow.

  • Mayo Jordanov

    Mayo Jordanov July 17th, 2008 @ 03:26 AM

    When property is a Float and no precision and scale is specified, precision seems to default to 10. This then triggers condition on line 35 in numeric_validator (http://github.com/sam/dm-more/tr...) which basically checks for a floating point number ending in ".0".

    Could both precision and scale be set to false for Float, which would fail line 31, triggering line 42, and resulting in the correct floating point detection?

    Failing spec attached.

  • Ana Nelson

    Ana Nelson July 17th, 2008 @ 03:40 AM

    The original problem I had was because I had a BigDecimal without specifying

    precision, scale so these defaulted to 10, 0 respectively. I would have had

    the same problem had I actually specified :scale => 0 and then tried to save

    a number with a decimal.

    I think it's easy to forget what your scale/precision has been set to,

    especially if it's set implicitly, but even if you set it yourself, so I

    like the idea of a very explicit validation error which says "you are

    violating scale X and precision y".

    It's possible to add something like:

    big_decimal = value.is_a?(BigDecimal)

    (or kind_of? if you prefer that but I find is_a? more natural)

    just before this line:

    http://github.com/sam/dm-more/tr...

    and then you can treat Float/BigDecimal differently further down. But, I

    don't know whether they need to be treated differently.

    2008/7/17 Lighthouse :

  • Mayo Jordanov

    Mayo Jordanov July 17th, 2008 @ 03:45 AM

    • Assigned user changed from “Sam Smoot” to “Dan Kubb (dkubb)”

    re-assigned to Dan, thanks man!

    The default of precision = 10 comes from DAO type map. I'm not sure why 10 was chosen (middle ground between 6 and 15??) From my experience picking precision of FP numbers is very tricky, as it totally depends on the platform and should be left for the underlying storage to deal with, IMHO.

  • Mayo Jordanov

    Mayo Jordanov July 17th, 2008 @ 04:08 AM

    Oops, sorry. I didn't mean to steal your ticket. I misread your example there, but I have to agree. Changing the error message like you suggest may be a very good idea. I'll start another ticket for my issue there.

    Sorry about that.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) July 17th, 2008 @ 01:00 PM

    The default of 10 for Float/BigDecimal was a rather arbitrary decision, similar to String being a length of 50. There's no right decision, what would work for you, may be too long or short for me.

    Code in DM has to work the same regardless of the adapter used, so there has to be a default set because otherwise the same model (without precision/scale specified) with one backend will work and another will return errors because it isn't able to support that precision and/or scale.

    The only alternative to picking a default would be to actually raise an exception if the precision and scale aren't specified for Float or BigDecimal properties.

  • Ana Nelson

    Ana Nelson July 17th, 2008 @ 01:10 PM

    I'm not saying that the defaults are bad or even that we should warn if no

    default is given.

    If you look at my 1-line patch, it just adds a more helpful error message in

    case the validation error is likely to be due to your choice of :scale. Will

    save a lot of debugging time without changing behaviour at all, other than

    the wording of the error message.

    I got burned by this in ActiveRecord a lot since I would forget to specify

    scale and my data would be truncated by the database. I'm much happier to

    get a validation error and not lose any information, I just think it should

    be a verbose validation error since forgetting to specify :scale is pretty

    common IMO and the current validation error doesn't lead you in the right

    direction to fix the problem.

    2008/7/17 Lighthouse :

  • Mayo Jordanov

    Mayo Jordanov July 17th, 2008 @ 03:49 PM

    Dan,

    I get the reasoning behind the String as well as BigDecimal (there should be a default on this one), but I don't agree with those defaults on a Float. Enough about there on here though, I should open a new ticket for that issue.

    I like Ana's fix to the error message that makes it much more clear to the developer what's wrong, but some may object it's not very friendly to users when you rely on the errors for user interface. One other solution may be to use logger.warn() to put a message about scale and precision into the logs.

  • Ana Nelson

    Ana Nelson July 17th, 2008 @ 05:20 PM

    Oh yeah. End users. :-) Well, I don't always have a logger running and

    I'm not sure I would have looked there in this case anyway since it

    wasn't a database issue.

    Saying "value is not a number" when you have typed what looks like a

    valid number to you is going to be confusing to end users as well as

    developers, so I think an error message which is more specific, i.e.

    "you have more decimal places in your input than %t was expecting"

    would be useful all around.

    On 17 Jul 2008, at 21:49, Lighthouse wrote:

  • Bernerd Schaefer

    Bernerd Schaefer July 22nd, 2008 @ 11:23 AM

    Mayo,

    Has this been fixed with your latest commit?

  • Mayo Jordanov

    Mayo Jordanov July 22nd, 2008 @ 11:51 AM

    No I haven't changed the error message. I can push this one through shortly.

  • Mayo Jordanov

    Mayo Jordanov July 22nd, 2008 @ 12:05 PM

    Dan resolved this in http://github.com/sam/dm-more/co..., so this issue can be closed now.

    Thanks.

  • Bernerd Schaefer

    Bernerd Schaefer July 22nd, 2008 @ 12:11 PM

    • State changed from “new” to “resolved”

    Excellent.

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 »

People watching this ticket

Pages