Nested loop with 'if' statements

Question

I am trying to loop through an array of options, and check each option to see if it matches a different option within a different array. If the two options match, do one thing, if they don't match, do another thing.

I have 3^2 available options, and it runs through nine times. It goes (true, false, false), (false, true, false), (false, false, true), which isn't ideal. Instead, I'd like to see something along the lines of: (true), (false), (true).

It sits in my frontend in an erb file. I realize that there shouldn't be logic there, and am happy for refactoring suggestions but did plan on moving it out after I figured out how to make the loop work.

Frontend Code:

<% @available_sites.each do |available_site| %>
  <% @sites.each do |site| %>
    <% if site.review_site == available_site.name %>
      <div class="site-row columns">
        <div class="column col-4 col-md-10 col-mx-auto">
          <div class="site-tile">
            <a href="<%= site.direct_review_url %>" style="background-image: url(<%= image_path("review_site_logos/#{site.review_site}.png")%>); background-size: cover;"></a>
          </div>
        </div>
        <div class="column col-4 col-md-10 col-mx-auto">
          <span>Average Rating: <%= site.average_rating %></span>
          <span>Review Site: <a href="<%= site.direct_review_url %>"><%= site.review_site %></a></span>

        </div>
        <div class="column col-4 col-md-10 col-mx-auto is-flex-centered request-row-buttons">
          <%= link_to 'Show', [site.location, site] %>
          <%= link_to 'Edit', [:edit, site.location, site] %>
          <%= link_to 'Destroy', [site.location, site], method: :delete, data: { confirm: 'Are you sure?' } %>
        </div>
      </div>
    <% elsif site.review_site != available_site.name %>
      <div class="site-row columns">
        <div class="column col-4 col-md-10 col-mx-auto">
          <div class="site-tile">
            <a href="<%= available_site.link_to_info %>" style="background-image: url(<%= image_path("review_site_logos/#{available_site.name}.png")%>); background-size: cover;"></a>
          </div>
        </div>
        <div class="column col-4 col-md-10 col-mx-auto">
          <span>Sign Up for <%= available_site.name %> Business Page: <%= available_site.link_to_signup %></span>
          <span>Review Site: <a href="<%= site.direct_review_url %>"><%= site.review_site %></a></span>

        </div>
        <div class="column col-4 col-md-10 col-mx-auto is-flex-centered">
          <%= link_to "Add #{available_site.name} to Review App", new_location_site_path(review_site: "#{available_site.name}"), class: 'btn btn-primary' %>
        </div>
      </div>
    <% end %>
  <% end %>
<% end %>

Relevant Controller Code sites_controller.rb - policy_scope is for authorization:

def index
    @sites = policy_scope(@location.sites)
    @available_sites = AvailableSite.all
  end

Relevent Schema Code

create_table "sites", force: :cascade do |t|
    t.bigint "location_id"
    t.string "review_site"
    t.string "direct_review_url"
    t.string "place_id"
    t.decimal "average_rating"
    t.jsonb "extra_data", default: {}, null: false
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
    t.integer "rating_count"
    t.index ["extra_data"], name: "index_sites_on_extra_data", using: :gin
    t.index ["location_id"], name: "index_sites_on_location_id"
  end
create_table "available_sites", force: :cascade do |t|
    t.string "name"
    t.string "link_to_info"
    t.string "link_to_signup"
    t.string "base_review_url"
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
    t.index ["name"], name: "index_available_sites_on_name"
  end

I'm looking for help/a new direction on handling nested loops.

BTW, you don't need to use an elsif here, just an else. You've already established that the names are not equal, so you don't have to check again. — Chris Hall
– Chris Hall, Commented Oct 4, 2018 at 2:02

Chris Hall · Accepted Answer · 2018-10-04 02:02:50Z

1

Just to make sure we're on the same page, I'm going to restate your problem:

You have a list of AvailableSites and you want to display different information depending on if you have a visible Site for it.

If that's accurate, you'll want to use find. Something like:

<% @available_sites.each do |available_site| %>
  <% site = @sites.find { |s| s.review_site == available_site.name } %>

  <% if site.present? %>
    <%# view fragment when you have a site %>
  <% else %>
    <%# view fragment when you don't have a site %>
  <% end %>
<% end %>

This will only output one set of elements per AvailableSite.

answered Oct 4, 2018 at 2:02

Chris Hall

91510 silver badges19 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Rich Over a year ago

I spent the whole day on this, and couldn't figure it out. Thanks so much for your help on it!

Andrew Schwartz · Accepted Answer · 2018-10-04 03:55:38Z

First, let's make it easier to parse and maintain by moving the two view partials into actual partial files. That way we have simply

<% @available_sites.each do |available_site| %>
  <% @sites.each do |site| %>
    <% if site.review_site == available_site.name %>
       <% render partial: "sites/site_available", locals: {site: site}>
    <% else %>
       <% render partial: "sites/site_unavailable", locals: {site: site}>
    <% end %>
  <% end %>
<% end %>

Much nicer to play around with!

Next, we don't need to render inside a nested loop; this is why you're getting 9 instead of 3 renderings. You need one outer loop over @sites. In each loop, it will (1) check availability, (2) render. That will look more like

<% @sites.each do |site| %>
  <% if site_available?(@site, @available_sites) %>
     <% render partial: "sites/site_available", locals: {site: site}>
  <% else %>
     <% render partial: "sites/site_unavailable", locals: {site: site}>
  <% end %>
<% end %>

where you have lots of options for what to actually do for, or instead of, site_available?, including:

Define it as a helper method, view code is exactly as above. Not my recommended approach, but it'll get the job done.
Use @available_sites.map(&:name).include?(@site.name). Now it's more obvious that this is a loop (include? will loop), but that the render is happening outside this loop. Somewhat inefficient, but so is AvailableSite.all so could be fine for now :-)
Use a presenter: this is just a simple Ruby class that will take a site as an arg to the initialize method, and have additional logic and attrs (such as @available) that you can add without polluting the model, view, or controller. If this gets more complex I always advocate for starting here, but it may be a bit heavy-handed for right now.

class AvailableSitePresenter
  def new(site, available_sites)
    @site = site
    @available = available_sites.map(&:name).include?(@site.name)
  end

  def available?
    @available
  end
end

Collectives™ on Stack Overflow

Nested loop with 'if' statements

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related