karmi / tire-contrib

Additions and extensions for the Tire gem
MIT License
65 stars 31 forks source link

Setting dynamic attributes after initialization #17

Closed SixiS closed 11 years ago

SixiS commented 11 years ago

Don't know if this is useful to anybody else, but I thought I would add a pull request anyway. Setting dynamic attributes after initialization.

karmi commented 11 years ago

@SixiS I'm really sorry, this has slipped me, I don't remember the original context, can you have a look at https://github.com/karmi/tire-contrib/blob/master/lib/tire/model/dynamic_persistence.rb whether it targets the same behaviour? Thanks.

SixiS commented 11 years ago

It is the same behaviour, I just extended it slightly.

https://github.com/karmi/tire-contrib/blob/master/lib/tire/model/dynamic_persistence.rb Allows one to set dynamic attributes at initialisation.

I needed to add dynamic attributes after initialisation (using the tire persistence) so I just added set_attribute and set_attributes methods.

karmi commented 11 years ago

Ah, OK, what about extending it further, and allow something like @book.entirely_new_attr = 'foo'? I'm wary about the set_attribute method name, people have to remember it etc. It would be better to smooth out the API, what do you think?

SixiS commented 11 years ago

I was a little hesitant to do this simply because the no SQL adapters I have come across have not implemented it. Mongoid for example uses @model.attributes[:something_dynamic] = "lorem ipson" for dynamic attributes.

I do like the idea tho, I will look at implementing it safely this afternoon.

Just as a first thought: The methods that need to be altered to make it seamless would be initialize, update_attributes, and then modifying method_missing to allow for entirely_new_attrs.

karmi commented 11 years ago

@SixiS Yeah, @mymodel.attributes[:something_new] = 'foo bar' sounds OK as well!

SixiS commented 11 years ago

I have implemented the new methods. Was simpler than I expected, and seems to work really nicely. I have left the set_attributes and set_attribute method as they were.

Let me know if there are any other tests I should write.

Thanks for looking into this pull request :)

karmi commented 11 years ago

@SixiS Finally merged, sorry about the holdup!, thanks!