Permalink
Browse files

stop using && for the short circuit side effect

  • Loading branch information...
1 parent 5d954b4 commit 992b3b5e065237986d6972e27fc109b05ab84e00 @tenderlove tenderlove committed Jun 28, 2011
Showing with 1 addition and 1 deletion.
  1. +1 −1 activerecord/lib/active_record/base.rb
@@ -2092,7 +2092,7 @@ def convert_number_column_value(value)
def populate_with_current_scope_attributes
self.class.scope_attributes.each do |att,value|
- respond_to?("#{att}=") && send("#{att}=", value)
+ send("#{att}=", value) if respond_to?("#{att}=")
@paul
paul Jun 28, 2011 Contributor

String interpolation can be slow as well, and so is doing it twice. Might I suggest:

method_name = "#{att}="
send(method_name, value) if respond_to?(method_name)

See: https://gist.github.com/1052128

end
end

4 comments on commit 992b3b5

@amalagaura

@paul Just came across this, why don't you submit a pull request with a link to your gist. But is interesting that the string interpolation is all over the place in Rails. I wonder if it is being accepted even though it is slower because it is more readable? I see string interpolation a lot to just add 1 character.

@paul
Contributor
paul commented on 992b3b5 Dec 8, 2011

It is definitely more readable. In the gist, however, you can see that a single string interpolation takes ~ 0.5 usec, so its really not worth the readability cost anywhere except the hot path, where it might get called thousands of times. Reading a few hundred records from the DB and making AR objects out of them is one of those hot paths, though.

@amalagaura

@paul That particular block is called on instantiation of every single attribute of every single record object! So it is worth optimizing.

I was looking at some examples and "#{attr}=" is only slightly more readable that attr+'='

I think you should put in your pull request with the most heavily optimized to_sym version with a link to your gist. It should be to_sym since it would have to do that twice too.

@tenderlove
Member

The to_sym part is being removed from ruby internals (since symbols don't get GC's). I'd recommend a pull request with this refactor but without the to_sym.

Please sign in to comment.