Skip to content

Instantly share code, notes, and snippets.

@ono
Created November 4, 2010 10:34
Show Gist options
  • Save ono/662314 to your computer and use it in GitHub Desktop.
Save ono/662314 to your computer and use it in GitHub Desktop.
A response for:
https://gist.github.com/660194
https://gist.github.com/660968
https://gist.github.com/661951
I don't discuss here whether you shouldn't change the Object class behaviour.
I will write my thoughts on an assumption which it were in ruby standard library.
1. Method name must be shorter at least
When you compare...
foo.try_scope {|a| a.map(&:downcase).map(&:capitalize)}
foo ? foo.map(&:downcase).map(&:capitalize) : nil
(foo and foo.map(&:downcase).map(&:capitalize))
I don't see advantage to use try_scope.
try_scope requires you more type and is more difficult to understand the logic for code reviewers.
Method name should be shorter like try or tap.
e.g.
foo.try {|a| a.map(&:downcase).map(&:capitalize)}
2. NilClass is special?
Nil Class is special? I often see my code like...
foo.blank? ? foo.map(&:downcase).map(&:capitalize) : nil
If I use try_scope only when I can expect nil but for anything else, the code will loose the consistency.
However try_scope will never be able to imply the logic behind that.
People will often have to check the document or definition to know how it exactly works.
3. About https://gist.github.com/660968
I thought that Makoto wants that ...
foo.try_scope.{|a| a.map{|b| b.capitalize}.map{|b| b.downcase}}
should be equivalent to
foo ? foo.map{|b| b.capitalize}.map{|b| b.downcase} : nil
So
['a', 'b', nil].try_scope{|a| a.map{|b| b.capitalize}.map{|b| b.downcase}}
is supposed to raise "NoMethodError: undefined method `capitalize' for nil:NilClass."
That's also @bitti insists in his comment bellow, I think.
https://github.com/rails/rails/commit/823b623fe2de8846c37aa13250010809ac940b57#diff-0
I wonder https://gist.github.com/661951 is even better than https://gist.github.com/660194.
@makoto
Copy link

makoto commented Nov 4, 2010

My test for the last one was wrong.
assert_equal(
['A', nil],
['a', nil].try_scope do |a|
a.map(&:upcase).map(&:downcase)
end
)

rather than

assert_equal(
  nil, 
  ['a', nil].try_scope do |a| 
    a.map(&:upcase).map(&:downcase)
  end  
)

With that in mind, I want something like this https://gist.github.com/663415 (this is not workable method, as it won't work as expected in threaded env).

What I really want is the ability to change class behaviour within a block, but not affecting others. Might be interesting topic to ask Sasada san tomorrow.

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