HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Another ||= question

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
questionanotherstackoverflow

Problem

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.

Solution

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

Code Snippets

evaluation ||= Evaluation.find(@user.id_of_required_evaluation)
return false unless isSelected

Context

StackExchange Code Review Q#13643, answer score: 8

Revisions (0)

No revisions yet.