Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

ActiveModel::Name does not inherit from string #5593

Merged
merged 1 commit into from

6 participants

@lukesarnacki

Like in ActiveModel::Errors we should favor composition over inheritance

@tenderlove tenderlove merged commit e8b5c8e into rails:master
@tenderlove

Do you know where these are being used? Eventually I'd like to get rid of the delegate methods.

@lukesarnacki

It made it for convenience, because it may be common to compare model name with a String. And without to_str test that was comparing "Anonynous" == model_name was failing.

@evanphx

Where should I send the $100?

@steveklabnik
Collaborator

Just for comparison, I was racing with this: steveklabnik@8f6a4b8

@jmazzi

@steveklabnik @lukesarnacki this is what I had jmazzi@0d85690.

@lukesarnacki removed the failing test I was trying to fix.

@lukesarnacki

@jmazzi I changed only test that asserts ActiveModel::Naming is a String. Not removed anything.

@jmazzi

I wasn't trying to imply the change you made was wrong. It was the proper change. I should have done the same.

@jmazzi

class Rkh < :trollface:
end

@tenderlove
Owner

Yet another reason I wish that :trollface: was a real unicode character.

@jmazzi

@tenderlove open an emergency feature request ticket for Ruby 2.0

@rkh

@tenderlove U+1F479 and U+1F47A come pretty close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 26, 2012
  1. ActiveModel::Name does not inherit from string

    Lukasz Sarnacki authored
This page is out of date. Refresh to see the latest.
View
2  activemodel/lib/active_model/lint.rb
@@ -78,7 +78,7 @@ def test_persisted?
def test_model_naming
assert model.class.respond_to?(:model_name), "The model should respond to model_name"
model_name = model.class.model_name
- assert_kind_of String, model_name
+ assert_kind_of ActiveModel::Name, model_name
assert_kind_of String, model_name.human
assert_kind_of String, model_name.singular
assert_kind_of String, model_name.plural
View
27 activemodel/lib/active_model/naming.rb
@@ -2,31 +2,36 @@
require 'active_support/core_ext/hash/except'
require 'active_support/core_ext/module/introspection'
require 'active_support/core_ext/module/deprecation'
+require 'active_support/core_ext/module/delegation'
require 'active_support/core_ext/object/blank'
module ActiveModel
- class Name < String
+ class Name
+ include Comparable
+
attr_reader :singular, :plural, :element, :collection,
- :singular_route_key, :route_key, :param_key, :i18n_key
+ :singular_route_key, :route_key, :param_key, :i18n_key,
+ :name
alias_method :cache_key, :collection
- def initialize(klass, namespace = nil, name = nil)
- name ||= klass.name
+ delegate :==, :===, :<=>, :=~, :"!~", :eql?, :to_s,
+ :to_str, :to => :name
- raise ArgumentError, "Class name cannot be blank. You need to supply a name argument when anonymous class given" if name.blank?
+ def initialize(klass, namespace = nil, name = nil)
+ @name = name || klass.name
- super(name)
+ raise ArgumentError, "Class name cannot be blank. You need to supply a name argument when anonymous class given" if @name.blank?
- @unnamespaced = self.sub(/^#{namespace.name}::/, '') if namespace
+ @unnamespaced = @name.sub(/^#{namespace.name}::/, '') if namespace
@klass = klass
- @singular = _singularize(self).freeze
+ @singular = _singularize(@name).freeze
@plural = ActiveSupport::Inflector.pluralize(@singular).freeze
- @element = ActiveSupport::Inflector.underscore(ActiveSupport::Inflector.demodulize(self)).freeze
+ @element = ActiveSupport::Inflector.underscore(ActiveSupport::Inflector.demodulize(@name)).freeze
@human = ActiveSupport::Inflector.humanize(@element).freeze
- @collection = ActiveSupport::Inflector.tableize(self).freeze
+ @collection = ActiveSupport::Inflector.tableize(@name).freeze
@param_key = (namespace ? _singularize(@unnamespaced) : @singular).freeze
- @i18n_key = self.underscore.to_sym
+ @i18n_key = @name.underscore.to_sym
@route_key = (namespace ? ActiveSupport::Inflector.pluralize(@param_key) : @plural.dup)
@singular_route_key = ActiveSupport::Inflector.singularize(@route_key).freeze
Something went wrong with that request. Please try again.