1

I am newbie. I am trying to develop a simple web application involving shops and candies where a shop can have many candies.

I have the following code in my shops/show.html.erb which displays list of candies twice.

<% i=0 %>
<% for candy in @shop.candies do %>
    <% i+=1 %>
    <%= i %> <%= candy.name  %>
<% end %>

<%= form_for([@shop, @shop.candies.build]) do |f| %>
            <%= render(:partial => 'form',
                       :locals => {:f => f, :header => "Add a Candy", 
                       :placeholder => "Enter a candy name"}) %>
<% end %>

<% i=0 %>
<% for candy in @shop.candies do %>
    <% i+=1 %>
    <%= i %> <%= candy.name  %>
<% end %>

My code in _form.html.erb for creating a new candy:

<%= f.text_field(:name, :placeholder=> placeholder, :class=>"form-control custom-input")%>
<%= button_tag( :class => "btn btn-primary mb-2 btn-custom btn-custom-sc") do %>
      <i class="fas fa-plus icon"></i>
<% end %>

Code of Shops Controller:

class ShopsController < ApplicationController

def show
       @shop = Shop.find(params[:id])
       @unshelved_candies = @shop.candies.unshelved_candies
    end
    private
        def shop_params
          params.require(:shop).permit(:name)
        end

end

Code of Candies Controller:

class CandiesController < ApplicationController

   def create
      @shop = Shop.find(params[:shop_id])
      @candy = @shop.candies.create(candy_params)
      redirect_to(shop_path(@shop))
    end

    private

            def candy_params
              params.require(:candy).permit(:name)
            end   
   end

end

When I run the code and view it on browser, I notice that it creates an empty candy in the second loop (not in database). However, when I remove the form for creating candies, it behaves as it should. I am unable to understand why it's looping one more time and displaying blank value. The output of the first loop is the correct one:

  1. Candy 1
  2. Candy 2
  3. Candy 3

And the output of second loop is:

  1. Candy 1
  2. Candy 2
  3. Candy 3
  4. [<---Empty. I am not inserting anything new to the database]

Can anybody tell me why it is displaying a blank value in second loop and how to prevent this extra iteration?

Improve this question

1 Answer 1

Reset to default
4

I believe the "extra" candy is the one you're instantiating here: 

<%= form_for([@shop, @shop.candies.build]) do |f| %>

The candy name for the new candy is nil, so you're getting the blank.

BTW, this: 

<% i=0 %>
<% for candy in @shop.candies do %>
  <% i+=1 %>
  <%= i %> <%= candy.name  %>
<% end %>

Strikes me as non-idiomatic ruby. I would expect to see something more like: 

<% @shop.candies.each.with_index(1) do |candy, index| %>
  <%= index %> <%= candy.name  %>
<% end %>

I guess a brute force way of making sure you don't get that extra candy would be to do something like:

<% @shop.candies.each.with_index(1) do |candy, index| %>
  <% unless candy.new_record? %>
    <%= index %> <%= candy.name  %>
  <% end %>
<% end %>

You might also try:

<%= form_for([@shop, @candy]) do |f| %>

Which I believe can be written: 

<%= form_for(@shop, @candy) do |f| %>

If you want to save yourself a couple of key strokes (they add up over time).

And then in your ShopsController do:

class ShopsController < ApplicationController

  def show
    @shop = Shop.find(params[:id])
    @candy = Candy.new
    @unshelved_candies = @shop.candies.unshelved_candies
  end

  private

  def shop_params
    params.require(:shop).permit(:name)
  end

end

This is also nice because it avoids: 

@shop.candies.build

Which requires that your view knows a lot about the relationship between shop and candies and also requires that your view interacts directly with the database.

Since you're apparently using nested routes, you might want to look at the shallow: true directive.

Also (this is not related to your question), you might want to be thoughtful about the Law of Demeter. I notice you do: 

@unshelved_candies = @shop.candies.unshelved_candies

Personally, I would do something more like:

@unshelved_candies = @shop.unshelved_candies

And in Shop, you might have something like: 

class Shop < ApplicationRecord

  def unselved_candies
    candies.unshelved
  end

end

And in Candy, something like: 

class Candy < ApplicationRecord

  class < self 

      def unshelved
        where(shelved: false) # or however you determine a candy is unshelved
      end

  end

end

Many people would make unshelved a scope, which is another way of doing the same thing.

This way, your ShopsController knows less about the mechanics of the relationships between shops and candies and shelved status. FWIW.

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

2 Comments

Thank you. Is there any way I can modify the form so that the extra candy is not created?
Updated answer.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.