The following construct is an ActiveSupport-ism:
returning(Foo.new) do |foo|
...
end
I don't especially like it, since it's both more verbose and less efficient
than the direct alternative:
foo = Foo.new
...
foo
It doesn't occur many times in Merb, so does anyone agree with me that it
should be removed?
I tried doing this (patch attached) and I find the code more readable
without. For example, in lib/merb/mixins/responder.rb
Before:
def <=>(entry)
returning((entry.quality <=> quality).to_s) do |c|
c.replace((index <=> entry.index).to_s) if c == '0'
end.to_i
end
After:
def <=>(entry)
c = (entry.quality <=> quality).to_s
c.replace((index <=> entry.index).to_s) if c == '0'
c.to_i
end
This second form also more clearly begs the question, why is the result from
<=> being converted to string and then back to integer? I don't know the
answer to that :-)
I believe the attached patch is OK - it doesn't break any specs. It doesn't
remove the definition of Object#returning itself, or its spec.
Regards,
Brian.
Index: lib/merb/gems.rb
===================================================================
--- lib/merb/gems.rb (revision 508)
+++ lib/merb/gems.rb (working copy)
@@ -27,16 +27,16 @@
end
def list_required(timing = :all)
- returning({}) do |required|
- with_manifest do |manifest|
- manifest.each do |gem,options|
- gem_when = (options["when"] || :after).to_sym
- if timing == :all || gem_when == timing
- required[gem] = options["version"]
- end
+ required = {}
+ with_manifest do |manifest|
+ manifest.each do |gem,options|
+ gem_when = (options["when"] || :after).to_sym
+ if timing == :all || gem_when == timing
+ required[gem] = options["version"]
end
end
end
+ required
end
def load_required(timing = :after)
Index: lib/merb/mixins/responder.rb
===================================================================
--- lib/merb/mixins/responder.rb (revision 508)
+++ lib/merb/mixins/responder.rb (working copy)
@@ -65,18 +65,18 @@
def self.parse(accept_header)
# parse the raw accept header into a unique, sorted array of
AcceptType objects
- returning( accept_header.split(/,/).enum_for(:each_with_index).map
do |entry,index|
+ list = accept_header.split(/,/).enum_for(:each_with_index).map do
|entry,index|
AcceptType.new(entry,index += 1)
- end.sort.uniq ) do |list|
- # firefox (and possibly other browsers) send broken default
accept headers.
- # fix them up by sorting alternate xml forms (namely
application/xhtml+xml)
- # ahead of pure xml types (application/xml,text/xml).
- if app_xml = list.detect{|e| e.super_range == 'application/xml'}
- list.select{|e| e.to_s =~ /\+xml/}.each { |acc_type|
- list[list.index(acc_type)],list[list.index(app_xml)] =
- list[list.index(app_xml)],list[list.index(acc_type)] }
- end
+ end.sort.uniq
+ # firefox (and possibly other browsers) send broken default accept
headers.
+ # fix them up by sorting alternate xml forms (namely
application/xhtml+xml)
+ # ahead of pure xml types (application/xml,text/xml).
+ if app_xml = list.detect{|e| e.super_range == 'application/xml'}
+ list.select{|e| e.to_s =~ /\+xml/}.each { |acc_type|
+ list[list.index(acc_type)],list[list.index(app_xml)] =
+ list[list.index(app_xml)],list[list.index(acc_type)] }
end
+ list
end
private
@@ -128,9 +128,9 @@
end
def <=>(entry)
- returning((entry.quality <=> quality).to_s) do |c|
- c.replace((index <=> entry.index).to_s) if c == '0'
- end.to_i
+ c = (entry.quality <=> quality).to_s
+ c.replace((index <=> entry.index).to_s) if c == '0'
+ c.to_i
end
def eql?(entry)
Index: lib/merb/template/erubis.rb
===================================================================
--- lib/merb/template/erubis.rb (revision 508)
+++ lib/merb/template/erubis.rb (working copy)
@@ -40,12 +40,12 @@
return @@erbs[path]
else
begin
- returning ::Erubis::MEruby.new(IO.read(path)) do |eruby|
- eruby.init_evaluator :filename => path
- if cache_template?(path)
- @@erbs[path], @@mtimes[path] = eruby, Time.now
- end
- end
+ eruby = ::Erubis::MEruby.new(IO.read(path))
+ eruby.init_evaluator :filename => path
+ if cache_template?(path)
+ @@erbs[path], @@mtimes[path] = eruby, Time.now
+ end
+ eruby
rescue Errno::ENOENT
raise "No template found at path: #{path}"
end
Index: lib/merb/mailer.rb
===================================================================
--- lib/merb/mailer.rb (revision 508)
+++ lib/merb/mailer.rb (working copy)
@@ -52,9 +52,9 @@
def initialize(o={})
self.config = :sendmail if config.nil?
o[:rawhtml] = o.delete(:html)
- @mail = returning MailFactory.new() do |m|
- o.each { |k,v| m.send "#{k}=", v }
- end
+ m = MailFactory.new()
+ o.each { |k,v| m.send "#{k}=", v }
+ @mail = m
end
end
_______________________________________________
Merb-devel mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/merb-devel