Search code examples
ruby-on-railsrubyiterationeachpolymorphic-associations

How can I combine two arrays into one by a common attribute?


How can I combine two arrays into one so that for example...

Day 1: Notes/form
Day 2: Notes/form
Day 3: Notes/form
Day 4: Notes/form
Day 5: Notes/form

If a user then creates a note on Day 3 it would look like...

Day 1: Notes/form
Day 2: Notes/form
Day 3: Notes/notes
Day 4: Notes/form
Day 5: Notes/form

The code monstrosity below tried to achieve the above, but the iteration is all off. If for example a note is created on Day 1 and Day 3 then the output will look like...

Day 1: Notes/notes # Shows both Day 1 and Day 3 note. Only want to show Day 1 note
Day 1: Notes/form
Day 2: Notes/form
Day 2: Notes/form
Day 3: Notes/notes # Shows both Day 1 and Day 3 note. Only want to show Day 3 note
Day 3: Notes/form
Day 4: Notes/form
Day 4: Notes/form
Day 5: Notes/form
Day 5: Notes/form

challenges/show

<% @challenge.dates_challenged.first(@challenge.days_challenged).each_with_index do |date, i| %>
  <% @notes.each do |note| %>
    Day <%= i + 1 %>
    <% if note.notes_date.strftime("%m/%d/%y") == date.strftime("%m/%d/%y") %>
      <%= render 'notes/notes' %>
    <% else %>
      <%= render 'notes/form', :date => date %>
    <% end %>
  <% end %>
<% end %>

To recap, a user creates a challenge. A challenge has the attribute days_challenged. The user chooses how many days will be challenged, i.e. 10, 15, 30, etc. For each of those days I want to show a notes/form. If the user then input a note on a day then the notes/form should be replaced on the show page by that note (no day should have more than one note associated with it).


Solution

  • You're doing too much inside the loop that you're using to find the day's note. This should be about right:

    <% @challenge.dates_challenged.first(@challenge.days_challenged).each_with_index do |date, i| %>
      Day <%= i + 1 %>
      <% if @notes.any? { |note| note.notes_date.strftime("%m/%d/%y") == date.strftime("%m/%d/%y") } %>
        <%= render 'notes/notes' %>
      <% else %>
        <%= render 'notes/form', :date => date %>
      <% end %>
    <% end %>
    

    This is still too much logic in the view.

    1. As photoionized commented, it would be easier if @notes were a Hash whose keys were Dates and whose values were Notes. You could then replace the any? with a hash lookup.
    2. It's unclear to me why you need to compare strftime("%m/%d/%y"). You should just be able to compare Dates. (If you can't do that because they're in different time zones or something, consider fixing that -- it'll cause confusion all over your application.)
    3. I'd extract dates_challenged.first(@challenge.days_challenged) into a method on Challenge named something like current_dates_challenged and use that in the view.