Created
November 4, 2010 10:34
-
-
Save ono/662314 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
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.