instance_variable_set (:@attributes, { :brand.to_s => "Apple" } ) overwrites all attributes instead of adding

Question

Background...

I'm writing a parser that looks at strings and tries to determine what products they might contain. I've created my own Token class to help.

class Token < ActiveRecord::BaseWithoutTable

  attr_accessor :regex
  attr_accessor :values

end

Example of a Token:

Token.new(:regex => /apple iphone 4/, :values => { :brand => "Apple", :product => "iPhone", :version => 4})

(where the hash keys all correspond to database columns in the products table.)

Here is the problem: In my Parser, when a Token is found, I attempt to add the associated values to a Product instance, like so:

token.values.each do |v|
   attrib, value = v[0], v[1]
   my_product.instance_variable_set(:@attributes, { attrib.to_s => value })
end

This works except that it seems as if I have to set all my attributes at the same time. If I do it in stages (ie: as I discover new tokens), it overwrites any unspecified attributes with nil. Am I missing something? Is there a better way to do this?

Is there any particular reason why you are using `instance_variable_set` or you just want to update the attribute represented by the `key` in the `Hash` returned by `token.values`? — Swanand, Dec 08 '10 at 08:15

score 1 · Answer 1 · answered Dec 08 '10 at 04:18

1

Modify the existing value (if it exists) instead of overwriting it:

if attr = my_product.instance_variable_get :@attributes
  attr[attrib.to_s] = value
else
  my_product.instance_variable_get :@attributes, { attrib.to_s => value }
end

The use of instance_variable_set seems sketchy; why don't you have an accessor on the Product itself?

class Product
  def attributes
    @attributes ||= {}
  end
end

...

token.values.each do |attr,v|
   my_product.attributes.merge!( attr.to_s => v )
end

answered Dec 08 '10 at 04:18

Phrogz

296,393
112
651
745

it works but can you explain what exactly "@attributes ||= {}" is doing? – vmardian Dec 08 '10 at 14:36
@vmardian This is the same as `@attributes = @attributes || {}`; it's a [common Ruby idiom](http://stackoverflow.com/questions/609612/ruby-code-explained/609853#609853) to say _"set this variable to this value unless it's already been set"_. (It doesn't actually work that way if you want to set `nil` or `false` explicitly, but that's a rare edge case.) – Phrogz Dec 08 '10 at 14:44

Swanand · Accepted Answer · 2010-12-08T08:18:34.673

If my_product is an active_record object, you can use write_attribute instead of instance_variable_set. Note that this will only write attributes i.e. database columns:

token.values.each do |v|
   attrib, value = v[0], v[1]
   my_product.write_attribute attrib.to_s, value # attrib.to_sym would work too
end

Also, if token.values returns a Hash, this is how you can iterate:

token.values.each do |k, v|
   my_product.write_attribute k, v
end

instance_variable_set (:@attributes, { :brand.to_s => "Apple" } ) overwrites all attributes instead of adding

2 Answers2