Topic: SImple if statement logic refactor

Is there a better way of writing out this logic?

@conditions = {}

if params[:category] == "all" or params[:category].blank?
#don't do anything at all
else
   @conditions[:sub_cat_option] = params[:cat]
end

Re: SImple if statement logic refactor

unless params[:category] == "all" or params[:category].blank?
   @conditions = {:sub_cat_option => params[:cat]}
end

Re: SImple if statement logic refactor

My preferred is similar to the above.

  
@conditions = {:sub_cat_option => params[:cat]} unless params[:category].blank? || params[:category] == "all"

I prefer the nil check to be first however which is why I reversed the parameter checks.  Suppose that the category could be "ALL" sometimes.  A maintainer in the future could simply slap a .downcase on the if statement causing a failure when the category is nil (nil.downcase = boomage...)

[code=wrong!]
# This code is wrong
@conditions = {:sub_cat_option => params[:cat]} unless params[:category].downcase == "all" || params[:category].blank?
[/code]

I also prefer the vertical bar 'or' operator.  Something about a subtle precedence difference, I believe, but I can not put my finger on it now.

Last edited by tonyennis (2008-10-22 12:49:58)

But if you want it / Then you must find it
But when you have it / There'll be no need for it