#23 ✓resolved
Sam Smoot

#attributes= in DM::Base

Reported by Sam Smoot | October 8th, 2007 @ 12:41 PM

Copied from the mailing-list (by Luke):

There is one aspect where I think it's lacking a little, but I wanted to get some thoughts from you guys before I write a patch.

I don't think DM should be aping AR too much, but this is one feature I do like; it lets you assign values to any instance variable, not just those that correspond to declared properties.

This lets you do handle things like password hashing. The accessors for the column holding the hash can be made private and we temporarily store the password in an instance variable. Then with a before_validation call back, hash the password and store it in the appropriate property.

So I was thinking of modifying the method, if the key doesn't match a column in the database, we just assign it to the corresponding instance variable.

def attributes=(values_hash) 
  table = session.schema[self.class] 

  values_hash.reject do |key, value| 
    protected_attribute? key 
  end.each_pair do |key, value| 
    if column = table[key] 
      instance_variable_set(column.instance_variable_name, value) 
    else 
      instance_variable_set("@#{key}", value) 
    end 
  end 
end

Comments and changes to this ticket

  • Sam Smoot

    Sam Smoot October 8th, 2007 @ 12:43 PM

    This is specced and applied in r300.

    # Mass-assign mapped fields.
    def attributes=(values_hash)
      table = session.schema[self.class]
      
      values_hash.reject do |key, value|
        protected_attribute? key
      end.each_pair do |key, value|
        if column = table[key]
          instance_variable_set(column.instance_variable_name, value)
        else
          send("#{key}=", value)
        end
      end
    end
    

    Note this doesn't send the attribute if there's a column present. That case still sets the ivar directly. I'm not sure what behaviour we want for sure... I'm thinking it should probably work the same, but that has problems with embedded values. Which I'll probably just develop a patch to...

  • Sam Smoot

    Sam Smoot October 20th, 2007 @ 07:56 PM

    • State changed from “open” to “resolved”

    Well, the original issue was resolved. If anyone has any issues with the current behaviour, feel free to make a suggestion and reopen the ticket.

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

Tags

Pages