Ruby - Nested IF statement syntax

Question

I'm trying to get this nested if statement to work but my syntax is wrong and I can't figure it out. I have a search box on a Rails 4 app and a sort dropdown. On the search results page, I want to sort product listings based on what the user selects in the sort dropdown. If a user doesn't enter a search term, I want a message to display. Here is my controller code.

If I remove conditions and just display a default sort, search results page displays fine so the error is in the if statement syntax.

def search
    if params[:search].present?

      if params[:sort] == "- Price - Low to High"
        @listings = ...
      elsif params[:sort] == "- Price - High to Low"
        @listings = ...
      elsif params[:sort] == "- New Arrivals"
        @listings = ...
      elsif params[:sort] == "- Random Shuffle"
        @listings = ...
      else
        @listings = ...
      end

    else
      flash[:notice] = "Please enter one or more search terms e.g. blue shirt."
    end

  end

What is the exact behavior of the code? Is the notice always shown? — Undo
– Undo ♦, Commented Oct 8, 2014 at 16:12
There are too many chained elsifs. It looks like you could make heavy use of "switch", that exists in Ruby in form of case/when: stackoverflow.com/questions/948135/… — D-side
– D-side, Commented Oct 8, 2014 at 16:14
You are checking that :search is present, but then "switching" on :sort. Is this behavior correct? — Jeff Price
– Jeff Price, Commented Oct 8, 2014 at 16:17
As @D-side pointed out this is in essence a switch statement (which are usually considered code-smell in ruby). Maybe identify what you are trying to do and we can help you from there because you can probably convert this to 1 or 2 lines. — engineersmnky
– engineersmnky, Commented Oct 8, 2014 at 16:24
@Undo the notice is shown only when the user clicks on the search button without entering a search term. This is a basic ecommerce app where users are searching for products to buy. The sort dropdown is on the search results page. — Moosa
– Moosa, Commented Oct 8, 2014 at 16:30

tadman · Accepted Answer · 2014-10-08 16:25:55Z

What you want here is a case statement, a switch from other languages like JavaScript and C:

def search
  if params[:search].present?
    @listings =
      case (params[:sort])
      when  "- Price - Low to High"
        ...
      when "- Price - High to Low"
        ...
      when "- New Arrivals"
        ...
      when "- Random Shuffle"
        ...
      else
        ...
      end
  else
    flash[:notice] = "Please enter one or more search terms e.g. blue shirt."
  end
end

In Ruby, the result of a case statement can be used to assign to a variable, so that eliminates a lot of the @listings = repetition. Same goes for an if.

The things you're comparing against here seem unusually specific. If you've got a drop-down listing that's being used to choose sort order, they should have more succinct values used internally, such as plh to represent "Price - Low to High", or even numerical codes. That means if the phrasing is changed your code still works.

engineersmnky · Accepted Answer · 2014-10-08 16:32:04Z

0

I would change your drop down to have values like price asc,price desc, arrival_date asc (not sure about random shuffle) then you could use.

def search
  if params[:search].present?
    sort_listings(params[:sort])
  else
    flash[:notice] = "Please enter one or more search terms e.g. blue shirt."
  end
end

def sort_listings(sort)
  @listings ||= ...
  @listing.order(sort)
end

answered Oct 8, 2014 at 16:32

engineersmnky

30.5k2 gold badges42 silver badges64 bronze badges

Collectives™ on Stack Overflow

Ruby - Nested IF statement syntax

2 Answers 2

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related