Another ||= question

Question

I promise that I've done my reading on this topic. I've seen many suggestions and critiques, but I haven't seen the argument below critiqued. I'm not trying to beat a dead horse, but I don't think anyone's offered this one up yet:

x ||= y
is functionally equivalent to
x = y unless x

The code below has been through 5 years of feature additions; hence the difference in coding style. An evaluation is a survey questionnaire. To determine which questions to present, first see if an evaluation ID is specified in the URL, then fall back on the evaluation the user should see (if any), then fall back on the evaluation configured for this site (if any).

evaluation = Evaluation.find(params[:evaluation_id]) if params[:evaluation_id]
evaluation ||= Evaluation.find(@user.id_of_required_evaluation)
evaluation = Evaluation.find(@site.id_of_required_evaluation) unless evaluation

This doesn't flow well visually. I'm at the point where I can use ||= or unless and don't believe there to be any difference between the two.

Therefore, my question is: Is there any reason to choose one of the below blocks over the other?

Option 1

evaluation = Evaluation.find(params[:evaluation_id]) if params[:evaluation_id]
evaluation ||= Evaluation.find(@user.id_of_required_evaluation)
evaluation ||= Evaluation.find(@site.id_of_required_evaluation) 

Option 2

evaluation = Evaluation.find(params[:evaluation_id]) if params[:evaluation_id]
evaluation = Evaluation.find(@user.id_of_required_evaluation) unless evaluation
evaluation = Evaluation.find(@site.id_of_required_evaluation) unless evaluation

My apologies to the dead horse who again finds himself being beaten and to those who have to watch. If this is a stupid question, please consider the horse and beat me instead.

Answer 1

I think it really just comes down to opinion.

I think using ||= is better in this case, it's easier to glance at the code and see what it's intent is. When using the inline if or unless, I have to scan the whole line to know what is going on.

evaluation ||= Evaluation.find(@user.id_of_required_evaluation)

That said, I do like the inline if and unless with returns, since it reads nicely.

return false unless isSelected

Answer 2

I would use neither of the proposed blocks and refactor it into into something like

class Evaluation
  # Determines and returns the effective evaluation. If an evaluation id
  # was passed in the url, that evaluation is returned. Otherwise, the
  # default evaluation for the +user+ or, if none, the +site+ is returned.
  #
  # @return [Evaluation] The effective evaluation for the parameters
  def find_evaluation(id_from_url, user, site)
    return find(id_from_url) if id_from_url
    find(@user.id_of_required_evaluation) || find(@site.id_of_required_evaluation)
  end
end

to be called as

evaluation = Evaluation.find_evaluation(params[:evaluation], @user, @site)

Answer 3

Well, it is subjective. :) so this is my understanding of the situation. Use the particular operator depending on what you want the user to understand immediately, that is try to be as specific as possible.

In this case, what you want a reader to notice is that the value of evaluation is updated only if the previous attempt failed to return a value. So I would go for ||= which is explicit about the value on the left.

The operators x if y and x unless y have a different purpose. They are used if the normal execution always goes through x however, for some reason (which is checked by a boolean expression y) you make an exception to this current flow and do not execute x. I do not think that your above workflow is suited for this from this point of view.