0

I have a basic eRuby each loop

 <% @product_images.each do |image|%>

        <% if @counter < 4 %>

           <% p 'Im in here' %>
        <% else %>

           <% return %>
        <% end %>

          <% @counter += 1 %>
          <% p @counter %>

 <% end %>

Inside my loop i have an if statement that checks if @counter is < than 4.

Controller code 

def show

     productId = params[:id]

     @product_images = ProductImage.where("product_id = ?", productId)

     @counter = 0
end

When i run this code it should return once the counter is greater than 4 but i get an error that says no implicit conversion of nil into String

This is pretty straight forward code i can't seem to figure out what i'm doing wrong. It seems like it's breaking in the line 

<% if @counter < 4 %>

Here is a picture of the error:

enter image description here

ricks
  • 3,154
  • 31
  • 51
  • 2
    Are you trying to limit the product images to no greater than four? If so, you should `@product_images` in the controller and then be done with all that logic in your view. (Or, perhaps, you're doing something else we can't see from the code you provide.) – jvillian Apr 19 '18 at 14:23
  • 1
    @jvillian yes i'm basically building an image gallery for products, so i need to show 4 and then a 5th one which when clicked will open a modal with all the images. I guess i could create an array of 4 and then then create another array with all of them to achieve this too. Thanks for the advice going to try it out. – ricks Apr 19 '18 at 14:26
  • @jvillian What you recommended worked perfectly, if you want submit it as the answer so i can accept it. Thanks again for the help. – ricks Apr 19 '18 at 14:49

1 Answers1

1

It looks like you are trying to limit the number of @product_images that are rendered in your view. Instead of using @counter, you should simply limit the number of @product_images in your controller, something like: 

def show
  @product = Product.find_by(id: params[:id])
  @product_images = @product.product_images.limit(4)
end

And then in your view, do something like: 

<% @product_images.each do |image| %>
  # do stuff
<% end %>

This, naturally, assumes that: 

class Product < ActiveRecord::Base 
  has_many :product_images
end

And: 

class ProductImage < ActiveRecord::Base
  belongs_to :product
end

You could put that logic back in the view, like: 

<% @product.product_images.limit(4).each do |image| %>
  # do stuff
<% end %>

And then your show action could be just:

def show
  @product = Product.find_by(id: params[:id])
end

But, I prefer leaving it in the controller to reduce the coupling between your views and your models.

jvillian
  • 19,953
  • 5
  • 31
  • 44