Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

stop subclassing string

  • Loading branch information...
commit 188cc90af9b29d5520564af7bd7bbcdc647953ca 1 parent e76ced0
Aaron Patterson tenderlove authored
13 actionpack/lib/action_view/template/resolver.rb
View
@@ -7,7 +7,7 @@ module ActionView
# = Action View Resolver
class Resolver
# Keeps all information about view path and builds virtual path.
- class Path < String
+ class Path
attr_reader :name, :prefix, :partial, :virtual
alias_method :partial?, :partial
@@ -19,9 +19,16 @@ def self.build(name, prefix, partial)
end
def initialize(name, prefix, partial, virtual)
- @name, @prefix, @partial = name, prefix, partial
- super(virtual)
+ @name = name
+ @prefix = prefix
+ @partial = partial
+ @virtual = virtual
end
+
+ def to_str
+ @virtual
+ end
+ alias :to_s :to_str
Andrés Mejía
andmej added a note

Just out of curiosity, why not define the to_s method directly instead of this alias?

Aaron Patterson Owner

We probably could. I defined to_str because we need that in order to coerce to a string. e.g.:

irb(main):001:0> class Foo; def to_str; 'hello'; end end
=> nil
irb(main):002:0> class Bar; def to_s; 'hello'; end end
=> nil
irb(main):003:0> File.join('/', Foo.new)
=> "/hello"
irb(main):004:0> File.join('/', Bar.new)
TypeError: can't convert Bar into String
    from (irb):4:in `join'
    from (irb):4
    from /Users/aaron/.local/bin/irb:12:in `<main>'
irb(main):005:0>

I actually would rather not implement to_s.

Andrés Mejía
andmej added a note

Got it. I didn't know about to_str and coercion. Thanks for the explanation!

Aaron Patterson Owner

No problem! :-D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
end
cattr_accessor :caching
2  actionpack/test/template/testing/null_resolver_test.rb
View
@@ -6,7 +6,7 @@ def test_should_return_template_for_any_path
templates = resolver.find_all("path.erb", "arbitrary", false, {:locale => [], :formats => [:html], :handlers => []})
assert_equal 1, templates.size, "expected one template"
assert_equal "Template generated by Null Resolver", templates.first.source
- assert_equal "arbitrary/path.erb", templates.first.virtual_path
+ assert_equal "arbitrary/path.erb", templates.first.virtual_path.to_s
assert_equal [:html], templates.first.formats
end
end
Andrés Mejía

Just out of curiosity, why not define the to_s method directly instead of this alias?

Aaron Patterson

We probably could. I defined to_str because we need that in order to coerce to a string. e.g.:

irb(main):001:0> class Foo; def to_str; 'hello'; end end
=> nil
irb(main):002:0> class Bar; def to_s; 'hello'; end end
=> nil
irb(main):003:0> File.join('/', Foo.new)
=> "/hello"
irb(main):004:0> File.join('/', Bar.new)
TypeError: can't convert Bar into String
    from (irb):4:in `join'
    from (irb):4
    from /Users/aaron/.local/bin/irb:12:in `<main>'
irb(main):005:0>

I actually would rather not implement to_s.

Andrés Mejía

Got it. I didn't know about to_str and coercion. Thanks for the explanation!

Please sign in to comment.
Something went wrong with that request. Please try again.