Rails core - May 2010 - Suggestion for improving value_to_boolean column conversion

Hi fantastic rails core developers.

Speaking of version 2.3.5:

I find it rather error_prone that values such that "some text" is
silently converted to false when stored in a boolean DB field.

Among other things it means that assign "some text" to boolean fields
on a model will not generate any validation messages (it will silently
be converted to false).

I think it is like silently swallowing exception without knowing what
to do about them.

I will suggest to modify the existing implementation of 
ActiveRecord::ConnectionAdapters::Column::value_to_boolean

to
ActiveRecord::ConnectionAdapters::Column.class_eval %q{
  def self.value_to_boolean(value)
    case value
    when *TRUE_VALUES.to_a: true
    when *FALSE_VALUES.to_a: false
    else nil
    end
  end
}

First of all it simplifies the implementation (getting rid of initial
if. secondly it will at least result in nil if value is neither
recognised among TRUE_VALUES nor FALSE_VALUES

Jarl

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

> I think it is like silently swallowing exception without knowing what
> to do about them.
>
> I will suggest to modify the existing implementation of
> ActiveRecord::ConnectionAdapters::Column::value_to_boolean
>
> to
> ActiveRecord::ConnectionAdapters::Column.class_eval %q{
>  def self.value_to_boolean(value)
>    case value
>    when *TRUE_VALUES.to_a: true
>    when *FALSE_VALUES.to_a: false
>    else nil
>    end
>  end
> }
>
> First of all it simplifies the implementation (getting rid of initial
> if. secondly it will at least result in nil if value is neither
> recognised among TRUE_VALUES nor FALSE_VALUES
The initial if is there deliberately to allow people with html forms
to transmit a nil value by sending a blank string.  But as for the
coerce to nil suggestion
that would be similarly silent for users.  People who didn''t set their
columns to NOT NULL would just start having NULL values showing up
where previously they were false.  Seems like this would replace one
slightly strange situation with another, and not necessarily improve
things




-- 
Cheers

Koz

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

Michael Koziarski <michael@koziarski.com> writes:
>> I think it is like silently swallowing exception without knowing what
>> to do about them.
>>
>> I will suggest to modify the existing implementation of
>> ActiveRecord::ConnectionAdapters::Column::value_to_boolean
>>
>> to
>> ActiveRecord::ConnectionAdapters::Column.class_eval %q{
>>  def self.value_to_boolean(value)
>>    case value
>>    when *TRUE_VALUES.to_a: true
>>    when *FALSE_VALUES.to_a: false
>>    else nil
>>    end
>>  end
>> }
>>
>> First of all it simplifies the implementation (getting rid of initial
>> if. secondly it will at least result in nil if value is neither
>> recognised among TRUE_VALUES nor FALSE_VALUES
>
> The initial if is there deliberately to allow people with html forms
> to transmit a nil value by sending a blank string.  But as for the
> coerce to nil suggestion
> that would be similarly silent for users.  People who didn''t set
their
> columns to NOT NULL would just start having NULL values showing up
> where previously they were false.  Seems like this would replace one
> slightly strange situation with another, and not necessarily improve
> things
I agree, I see your point. Optimally I would suggest raising an
ArgumentError opposed to returning nil.. But there seems to be a
return-nil-instead-of-raise-exception practice in the coerce
functions. so I suggested "else nil" to follow that convention.

Currently the implementation of value_to_boolean does not follow the
convention of returning nil, where input is not coerceable. Many other
coerce functions return nil in these situation, see
ActiveRecord::ConnectionAdapters::Column.string_to_date("bullshit")
=> nil

But I also see that this "convention" is not general for all coerce
functions:
ActiveRecord::ConnectionAdapters::Column.value_to_decimal("asdfasdf")
=> #<BigDecimal:7fa16c93fe08,''0.0'',9(9)>

Actually my frustration was this:
http://groups.google.com/group/rubyonrails-core/browse_thread/thread/69b8a475f60c3f24#

I assume now that I see value_to_decimal returns 0.0 as default value,
I would have the same frustration about

validates_inclusion_of :decimal_field, :in => [0]

I think I will raise a bug on the more high level problem, and let the
core developers decided how it should be implemented.

Jarl

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.