Skip to content
This repository

Nice and easy delegation to the class #7613

Merged
merged 1 commit into from over 1 year ago

8 participants

Marc-André Lafortune José Valim Peter Brown Rafael Mendonça França Santiago Pastorino Jeremy Kemper Toby Ovod-Everett Dmitry Vorotilin
Marc-André Lafortune

Simple patch to allow delegation to class. With it, one can:

class FooFeatureController < ActionController::Base
  def self.feature_name
    controller_name.sub /_feature$/, ''
  end
  # In the same way that controller_name is available from the instances too:
  delegate :feature_name, :to => :class
end

Without it, one gets a relatively obscure syntax error message.

rails/activesupport/lib/active_support/core_ext/module/delegation.rb:142:in `module_eval': test/core_ext/module_test.rb:37: syntax error, unexpected '.' (SyntaxError)
            class.table_name(*args, &block)    ...
                  ^
test/core_ext/module_test.rb:39: syntax error, unexpected '.'
            if class.nil?                         ...
                     ^
test/core_ext/module_test.rb:44: syntax error, unexpected keyword_end, expecting $end
          end                                                 # end
             ^

It's already possible to use delegate :foo, :to => 'self.class' but that is less obvious (and also won't give the expected result with :prefix => true)

José Valim
Owner

Thanks, but I am not sure if we should worry about this. If we pass any other keyword, it will also fail. So we should list them all (nah)? So in the same way the developer cannot simply invoke "class" in his code, I think it is fine for him to know he cannot simply pass "class" to delegate.

Marc-André Lafortune

Thanks for commenting.

Note that it's easy to imagine an implementation of delegate that would use define_method and send and thus would work out of the box for :class. Actually that's why I thought it would work.

Which other keyword that is commonly used as a method name are you referring to?

What's the downside of my proposal?

Peter Brown

Why can't you just use:

delegate :feature_name, :to => 'self.class'
Rafael Mendonça França

For me delegate :feature_name, :to => 'self.class' is more obvious.

delegate :feature_name, :to => :class for me is too magical and I prefer to have less magic in the Rails code if we can.

Marc-André Lafortune

@beerlington wrote:

Why can't you just use: delegate :feature_name, :to => 'self.class'

Anyone that prefers that can (and still could if my patch is accepted), at least as long as one doesn't try :prefix => true. That's what I wrote at the end of my original request, BTW.

The question is

@rafaelfranca wrote:

For me delegate :feature_name, :to => 'self.class' is more obvious.
delegate :feature_name, :to => :class for me is too magical and I prefer to have less magic in the Rails code if we can.

We agree on less magic :-) but maybe not on what magic is. I believe the fact that to => 'self.class' actually works relies on the implementation using class_eval and not define_method. It's also not made explicit in the documentation that to: 'any_expression_here' will work and I would discourage anyone from writing to: '(customer.address || {})', say, even though it currently works.

I like the fact that :to => :class can be made to work in whichever way it is implemented (define_method or class_eval), while :to => 'self.class' requires an implementation that accepts an arbitrary string expression and uses class_eval.

I definitely won't fight to death over this, though! If I'm the only one to like it, I'll just resort to using :to => 'self.class', no problem :-)

Rafael Mendonça França

@marcandre if you are going to change delegate to use define_method so I'm fine with this change.

BTW I'm not giving :-1: for this feature. I just expressed my opinion about the actual implementation.

Lets hear more opinions.

cc/ @jeremy @fxn @spastorino @carlosantoniodasilva

Santiago Pastorino
Owner

I don't even like delegate

Jeremy Kemper
Owner

I like it. class is a message like any other; we're just used to having to do self.class due to syntax ambiguity. It works fine with send.

>> "foo".send(:class)
=> String
Jeremy Kemper jeremy merged commit 0a7b131 into from
Jeremy Kemper jeremy closed this
bUg. slbug referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
bUg. slbug referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Toby Ovod-Everett

Just a :+1: from those of us out here in user land. I ran into this issue 20 minutes ago and hacked up a quick monkey patch for my app running on 3.2.9. I was going to try my hand at my first PR for Rails when I decided to check Edge and, lo and behold, there it was!

I concur with @jeremy and @marcandre. I actually had to go look at the source to discover that to: 'self.class' was a workaround for 3.2.9 (I had figured out that class_eval was probably being used, but still didn't know the shape of the code on the inside), and I think it's prettier to be able to use to: :class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Sep 11, 2012
Marc-André Lafortune Nice and easy delegation to the class ad5f18e
This page is out of date. Refresh to see the latest.
3  activesupport/lib/active_support/core_ext/module/delegation.rb
@@ -123,6 +123,9 @@ def delegate(*methods)
123 123
     file, line = caller.first.split(':', 2)
124 124
     line = line.to_i
125 125
 
  126
+    to = to.to_s
  127
+    to = 'self.class' if to == 'class'
  128
+
126 129
     methods.each do |method|
127 130
       # Attribute writer methods only accept one argument. Makes sure []=
128 131
       # methods still accept two arguments.
11  activesupport/test/core_ext/module_test.rb
@@ -34,6 +34,12 @@ class Someone < Struct.new(:name, :place)
34 34
   delegate :street, :city, :to_f, :to => :place
35 35
   delegate :name=, :to => :place, :prefix => true
36 36
   delegate :upcase, :to => "place.city"
  37
+  delegate :table_name, :to => :class
  38
+  delegate :table_name, :to => :class, :prefix => true
  39
+
  40
+  def self.table_name
  41
+    'some_table'
  42
+  end
37 43
 
38 44
   FAILED_DELEGATE_LINE = __LINE__ + 1
39 45
   delegate :foo, :to => :place
@@ -111,6 +117,11 @@ def test_delegation_to_instance_variable
111 117
     assert_equal "DAVID HANSSON", david.upcase
112 118
   end
113 119
 
  120
+  def test_delegation_to_class_method
  121
+    assert_equal 'some_table', @david.table_name
  122
+    assert_equal 'some_table', @david.class_table_name
  123
+  end
  124
+
114 125
   def test_missing_delegation_target
115 126
     assert_raise(ArgumentError) do
116 127
       Name.send :delegate, :nowhere
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.