brewster / elastictastic

Object-document mapper and lightweight API adapter for ElasticSearch
MIT License
88 stars 13 forks source link

Assigning numerical values #17

Open kostia opened 12 years ago

kostia commented 12 years ago

Hello,

there is no automatically parsing on assigning numerical values yet.

Following works in ActiveRecord:

class Company < ActiveRecord::Base
  attr_accessible :sort_key
end

c = Company.new
c.attributes = {sort_key: '13'}
c.sort_key #=> 13
c.sort_key.class #=> Fixnum

Which doesn't yet work in `Elastictastic':

class Company
  include Elastictastic::Document
  field :sort_key, type: :integer
end

c = Company.new
c.attributes = {sort_key: '13'}
c.sort_key #=> '13'
c.sort_key.class #=> String

That makes it really unhandy to assign numerical values from request params (for even integer values are submitted as strings).

Would be great if it would work!

Thanx! Kostia

outoftime commented 12 years ago

Good call. Since checking for type and potentially casting the value will impose a performance penalty, my inclination would be to only do this during mass-assignment; presumably if one's application code is assigning a value directly, it will already be passing the correct type. Does this make sense?

Assuming that's the approach, I'll probably factor the mass-assignment logic into a separate module, which would be included in Elastictastic::Document but not the bare-metal Elastictastic::BasicDocument.

kostia commented 12 years ago

Would make totally sense for me. What about boolean attributes? Here's how ActiveRecord behaves:

c = Company.new
c.online = true
c.online.class #=> TrueClass
c.online = '0'
c.online.class #=> FalseClass
c.online = 'foo'
c.online.class #=> FalseClass
c.online = 1
c.online.class #=> TrueClass
c.online = '1'
c.online.class #=> TrueClass

Short: everything except true, "1" and 1 results in false. One exception is nil: assigning nil would result in nil.

outoftime commented 12 years ago

Yep, I was planning on just casting all types to their respective Ruby classes (times would be another big one).

kostia commented 12 years ago

Could you please extract mass-assignment. Then I would write the casting logic.