Search code examples
rubycoding-style

How do you assign a variable with the result of a if..else block?


I had an argument with a colleague about the best way to assign a variable in an if..else block. His orignal code was :

@products = if params[:category]
  Category.find(params[:category]).products
else
  Product.all
end

I rewrote it this way :

if params[:category]
  @products = Category.find(params[:category]).products
else
  @products = Product.all
end

This could also be rewritten with a one-liner using a ternery operator (? :) but let's pretend that product assignment was longer than a 100 character and couldn't fit in one line.

Which of the two is clearer to you? The first solution takes a little less space but I thought that declaring a variable and assigning it three lines after can be more error prone. I also like to see my if and else aligned, makes it easier for my brain to parse it!


Solution

  • As an alternative to the syntax in badp's answer, I'd like to propose:

    @products = 
      if params[:category]
        Category.find(params[:category]).products
      else
        Product.all
      end
    

    I claim this has two advantages:

    1. Uniform indentation: each level of logical nesting is indented by exactly two spaces (OK, maybe this is just a matter of taste)
    2. Horizontal compactness: A longer variable name will not push the indented code past the 80 (or whatever) column mark

    It does take an extra line of code, which I would normally dislike, but in this case it seems worthwhile to trade vertical minimalism for horizontal minimalism.

    Disclaimer: this is my own idiosyncratic approach and I don't know to what extent it is used elsewhere in the Ruby community.

    Edit: I should mention that matsadler's answer is also similar to this one. I do think having some indentation is helpful. I hope that's enough to justify making this a separate answer.