Hello, I''m looking for feedback on a possible pull-request to rails. Here''s the idea: right now any time an ActiveModel::Name is needed, it is accessed through model.class.model_name. In other words, currently, it''s something like this: # Somewhere in rails model.class.model_name class MyModel def self.model_name ActiveModel::Name.new(self) endend And I''d like it to be like this: # Somewhere in rails model.model_name class MyModel def self.model_name ActiveModel::Name.new(self) end delegate :model_name, to: :''self.class''end Here''s a gist of what inspired me to try going this direction: https://gist.github.com/3096204 I started with this: https://github.com/amiel/rails/commit/a64e5b4ccf9e851b06cd2eb64d824a9c2043c6e7, and realized that maybe I should get some feedback first. My initial questions are: 1. Do you think this is a good idea? Also, I''m surprised this hasn''t been done already, have I missed a rejected pull request? 2. How should the instance method model_name be added to ActiveModel classes? The class method model_name is provided by extending ActiveModel::Naming. Is it appropriate to use the extended hook in ActiveModel::Naming to either include another module or use define_method? 3. What are some arguments for or against the change? I''m mostly looking for arguments for the change to include in the pull request description, but I''m also curious what the arguments against would be. This was all posted to a gist here: https://gist.github.com/3189699, feel free to comment there or reply here. Thanks, -Amiel -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-core/-/ceX5k_DCZgkJ. 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.
Carlos Antonio da Silva
2012-Jul-28 23:15 UTC
Re: Looking for feedback on a change to ActiveModel
I''d say it''s unnecessary cluttering to the API, and could collide with someone using `model_name` in any sort of model already out there. I think you may be overlooking the way to create your "super simple object", probably one of these two would work quite fine: Class.new(OpenStruct) do def self.model_name @model_name ||= ActiveModel::Name.new(self) end def self.name "User" end end Class.new(OpenStruct) do def self.name "User" end extend ActiveModel::Naming end Also, I don''t see why you couldn''t just create a real class and extend Naming on it, should work just fine :). -- At. Carlos Antonio On Friday, July 27, 2012 at 7:25 PM, Amiel Martin wrote:> > Hello, I''m looking for feedback on a possible pull-request to rails. > > > Here''s the idea: right now any time an ActiveModel::Name is needed, it is accessed through model.class.model_name. > > > In other words, currently, it''s something like this: > > # Somewhere in rails model.class.model_name class MyModel def self.model_name ActiveModel::Name.new(self) end end > > > And I''d like it to be like this: > > # Somewhere in rails model.model_name class MyModel def self.model_name ActiveModel::Name.new(self) end delegate :model_name, to: :''self.class'' end > > > Here''s a gist of what inspired me to try going this direction: https://gist.github.com/3096204 > > > I started with this: https://github.com/amiel/rails/commit/a64e5b4ccf9e851b06cd2eb64d824a9c2043c6e7, and realized that maybe I should get some feedback first. > > > My initial questions are: > > Do you think this is a good idea? Also, I''m surprised this hasn''t been done already, have I missed a rejected pull request? > How should the instance method model_name be added to ActiveModel classes? The class method model_name is provided by extending ActiveModel::Naming. Is it appropriate to use the extended hook inActiveModel::Naming to either include another module or use define_method? > What are some arguments for or against the change? I''m mostly looking for arguments for the change to include in the pull request description, but I''m also curious what the arguments against would be. > > > This was all posted to a gist here: https://gist.github.com/3189699, feel free to comment there or reply here. > > Thanks, > > -Amiel > > -- > You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. > To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-core/-/ceX5k_DCZgkJ. > To post to this group, send email to rubyonrails-core@googlegroups.com (mailto:rubyonrails-core@googlegroups.com). > To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com (mailto:rubyonrails-core+unsubscribe@googlegroups.com). > For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.-- 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.