Skip to content

Commit

Permalink
Merge pull request #5593 from lukesarnacki/activemodel_name_compositi…
Browse files Browse the repository at this point in the history
…on_over_inheritance

ActiveModel::Name does not inherit from string
  • Loading branch information
tenderlove committed Mar 26, 2012
2 parents 1d59caa + 72cbccb commit e8b5c8e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 12 deletions.
2 changes: 1 addition & 1 deletion activemodel/lib/active_model/lint.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def test_persisted?
def test_model_naming def test_model_naming
assert model.class.respond_to?(:model_name), "The model should respond to model_name" assert model.class.respond_to?(:model_name), "The model should respond to model_name"
model_name = model.class.model_name model_name = model.class.model_name
assert_kind_of String, model_name assert_kind_of ActiveModel::Name, model_name

This comment has been minimized.

Copy link
@Peeja

Peeja Mar 28, 2012

Contributor

Is this necessary? Seems like we could just let the model_name be duck-typed. We're already asserting that human, singular, etc. are Strings.

This comment has been minimized.

Copy link
@jeremy

jeremy Mar 28, 2012

Member

👍 to assert that all these respond to to_s rather than asserting their type

This comment has been minimized.

Copy link
@drogus

drogus Mar 28, 2012

Member

@jeremy wouldn't it be better to check to_str? Everything responds to to_s ;)

Object.new.respond_to?(:to_s)

This comment has been minimized.

Copy link
@tenderlove

tenderlove Mar 28, 2012

Author Member

Why don't we just check that calling to_s does what it's supposed to do? Tell rather than ask.

This comment has been minimized.

Copy link
@Peeja

Peeja Mar 28, 2012

Contributor

Well, we'd be changing the contract then; they'd only work in a to_s'd context, like string interpolation, not necessarily in, say, concatenation with +. Might be an academic point.

Also, I'm not sure how it would be telling rather than asking. Just seems like a different ask.

assert_kind_of String, model_name.human assert_kind_of String, model_name.human
assert_kind_of String, model_name.singular assert_kind_of String, model_name.singular
assert_kind_of String, model_name.plural assert_kind_of String, model_name.plural
Expand Down
27 changes: 16 additions & 11 deletions activemodel/lib/active_model/naming.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -2,31 +2,36 @@
require 'active_support/core_ext/hash/except' require 'active_support/core_ext/hash/except'
require 'active_support/core_ext/module/introspection' require 'active_support/core_ext/module/introspection'
require 'active_support/core_ext/module/deprecation' require 'active_support/core_ext/module/deprecation'
require 'active_support/core_ext/module/delegation'
require 'active_support/core_ext/object/blank' require 'active_support/core_ext/object/blank'


module ActiveModel module ActiveModel
class Name < String class Name
include Comparable

attr_reader :singular, :plural, :element, :collection, 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 alias_method :cache_key, :collection


def initialize(klass, namespace = nil, name = nil) delegate :==, :===, :<=>, :=~, :"!~", :eql?, :to_s,
name ||= klass.name :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 @klass = klass
@singular = _singularize(self).freeze @singular = _singularize(@name).freeze
@plural = ActiveSupport::Inflector.pluralize(@singular).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 @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 @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) @route_key = (namespace ? ActiveSupport::Inflector.pluralize(@param_key) : @plural.dup)
@singular_route_key = ActiveSupport::Inflector.singularize(@route_key).freeze @singular_route_key = ActiveSupport::Inflector.singularize(@route_key).freeze
Expand Down

1 comment on commit e8b5c8e

@Peeja
Copy link
Contributor

@Peeja Peeja commented on e8b5c8e Mar 28, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huzzah! Another win for OOP!

Please sign in to comment.