Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #4134 (reopened enhancement)

Opened 2 years ago

Last modified 1 year ago

[PATCH] delegate method ignore on nil

Reported by: court3nay Assigned to: David
Priority: normal Milestone: 2.x
Component: ActiveSupport Version: edge
Severity: normal Keywords: tiny
Cc: devslashnull@gmail.com, rails@zerosum.org

Description

if you use delegate on an association that's empty, it crapxors out. 

delegate :name, :to => 'monkeys.first'

is fine, but ideally i'd either 

delegate :name, :to => 'monkeys.first', :unless => monkeys.first.nil?

which gets tricky due to compile-time evaluation of the unless/if clause.., or, 

delegate :name, :to => 'monkeys.first', :ignore_nil => true

I know y'all will have your own preferred syntax (not really happy with any of those), so here's how I have it locally... 

Index: /Volumes/rails/ba_typo/vendor/rails/activesupport/lib/active_support/core_ext/module/delegation.rb
===================================================================
--- activesupport/lib/active_support/core_ext/module/delegation.rb	(revision 3811)
+++ activesupport/lib/active_support/core_ext/module/delegation.rb	(working copy)
@@ -4,11 +4,11 @@
     unless options.is_a?(Hash) && to = options[:to]
       raise ArgumentError, "Delegation needs a target. Supply an options hash with a :to key"
     end
    
     methods.each do |method|
       module_eval(<<-EOS, "(__DELEGATION__)", 1)
         def #{method}(*args, &block)
-          #{to}.__send__(#{method.inspect}, *args, &block)
+          #{to}.__send__(#{method.inspect}, *args, &block) #{options[:ignore_nil]? "unless #{to}.nil?":""}
         end
       EOS
     end

Attachments

delegate_to_nil.patch (1.2 kB) - added by zapnap on 05/30/07 17:40:41.
return nil if delegation target is nil instead of blowing up with a NoMethodError

Change History

03/09/06 02:04:01 changed by devslashnull@gmail.com

  • cc set to devslashnull@gmail.com.

Could you explain a bit more when this would be useful? In your example, you'll end up with a method that looks like this: 

def name(*args, &block)
  monkeys.first.__send__(:name, *args, &block) unless monkeys.first.nil?
end

This will still give you an error when monkeys is nil.

03/09/06 17:46:50 changed by court3nay

it won't error if monkeys.first is nil. monkeys won't be nil because it's an association, and you'd be an idiot (heheh) to refer to an association you haven't actually made yet. ok, more explanation:

banana has_many monkeys

in this case, 'monkeys' is an association object, which is definitely not nil; you can do things like banana.monkeys.find(3) you can also do banana.monkeys.first, but if there are no associated monkeys, it'll be nil.

03/09/06 17:56:58 changed by devslashnull@gmail.com

So basically you want to be able to do things like name.favorite_tree and have that be nil, even though the object that name delegates to is nil?

03/09/06 18:12:55 changed by court3nay

no.. the delegate method just uses that one _field_ from the delegated object, so banana.name == banana.monkey[0].name unless monkey[0].nil? Without the patch, it errors if you're missing a monkey; you should be able to specify that.

All my patch does is allow you to gently escape the evil clutches of a missing monkey.

04/01/06 02:06:36 changed by spam@nickm.org

  • version changed from 1.0.0 to 1.1.0.

Yes, this is quite helpful. In my case, I have a model that may belong to another model. But since it doesn't always have to belong to it, the delegate methods blow up if the parent doesn't exist.

I think this allow_nil thing shouldn't even be an option, and this is how it should behave by default. That way, regardless of things, the delegate methods get defined (just returning nil).

05/29/07 05:34:38 changed by josh

  • status changed from new to closed.
  • resolution set to wontfix.

Closing stale ticket. Please reopen if your still interested.

05/30/07 17:40:41 changed by zapnap

  • attachment delegate_to_nil.patch added.

return nil if delegation target is nil instead of blowing up with a NoMethodError

05/30/07 17:43:23 changed by zapnap

  • status changed from closed to reopened.
  • cc changed from devslashnull@gmail.com to devslashnull@gmail.com, rails@zerosum.org.
  • component changed from ActiveRecord to ActiveSupport.
  • summary changed from delegate method ignore on nil to [PATCH][TINY] delegate method ignore on nil.
  • version changed from 1.1.0 to edge.
  • milestone set to 1.x.
  • keywords set to patch, tiny.
  • resolution deleted.

I think this is important and would like to see it addressed. I came to a similar conclusion, but couldn't think of any reason an ignore_nil option was needed. Please feel free to shoot me down and propose a counter-argument. Attaching my patch to r6908 with test.

05/30/07 18:15:03 changed by protocool

I'm not sure that I agree with this feature.

You can get the short-circuit behavior you're looking for with the following construct: 

delegate :foo, :wibble, :to => '(bar or return nil)'

Some people will probably find sticking an expression in the :to a bit ugly but it does allow you greater freedom than just "always return nil if :to evaluates to nil": 

delegate :foo, :wibble, :to => '(bar or return "bar not there")'

You could probably make a (strong) argument that it would be clearer to write custom accessors - but personally I like the way delegate() works right now.

06/04/07 04:34:05 changed by josh

  • keywords changed from patch, tiny to tiny.
  • summary changed from [PATCH][TINY] delegate method ignore on nil to [PATCH] delegate method ignore on nil.

(Using tiny keyword instead)