3 ms·

So, I'm a Ruby neophyte. Why did adding parenthesis speed that line up so much?

• jbverschoor 13y ago
  @owner ||= User.find_by_login(params[:user_id]) if params[:user_id].present? means: if params[:user_id].present? if not @owner @owner = User.find_by_login(params[:user_id]) end end @owner ||= (User.find_by_login(params[:user_id]) if params[:user_id].present?) means: if not @owner if params[:user_id].present? @owner = User.find_by_login(params[:user_id]) end end So everytime someone called owner() to get the owner of a repo, the regex was checked
• Moto7451 13y ago
  I haven't done too much Ruby myself, but the line looks sufficiently Perl like I believe the subtle change converted the first line (rolled out) from: (excusing my Perl-Ruby mashup) if (params[:user_id].present){ if (!@owner){ @owner = User.find_by_login(params[:user_id]); } } to: if (!@owner){ if (params[:user_id].present){ @owner = User.find_by_login(params[:user_id]); } } Now the call params[:user_id].present is only made if @owner lacks a value.
• Mithaldu 13y ago
  I don't write Ruby, but it's close enough to Perl that i can tell you. Precedence is the reason. The first line with more explicit precedence is this: if( params[:user_id].present?) { @owner ||= User.find_by_login(params[:user_id]) } The second line is this: @owner ||= ( User.find_by_login(params[:user_id]) if params[:user_id].present? ) In other words, the first version always ran the present? method, while the second version only did so if @owner was false.
• deleted 13y ago
  [deleted]
• MaulingMonkey 13y ago
  Rewritten in longer form, assuming I've made no mistakes, the two versions are equivalent to... if params[:user_id].present? if !@owner @owner = User.find_by_login(params[:user_id]) end end vs if !@owner if params[:user_id].present? @owner = User.find_by_login(params[:user_id]) end end Checking the member variable truthiness is presumably faster than a dictionary lookup followed by a call to "present?". As @owner is probably frequently/always true after the first call (it will be unless find_by_login returns false/nil/? or another method resets @owner, given the same instance), this leads to fewer dictionary lookups and present? calls, resulting in a performance increase. EDIT: Well that was an amusing spam of simultaneous answers.
• stock_toaster 13y ago
  Assignment has higher precedence than if, so the line originally would have evaluated as: (@owner ||= User.find_by_login(params[:user_id])) if params[:user_id].present? Thus always testing the if, and then conditionally setting @owner. The new line checks the existence of @owner before evaluating the if by using parens to set precedence.