Rails Carousel View

Question

In my web site, I created a carousel. Just now, I realized that I most likely repeated myself with this code. I would like to learn how to DRY up this code.

Here is my view file (.html.erb) format:

<div class="carousel-inner">
  <div class="item active">
    <%= link_to image_tag(image_path("#{@title}/1.jpg")), home_path %>
  </div>

  <div class="item">
    <%= link_to image_tag(image_path("#{@title}/2.jpg")), home_path %>
  </div>

  <div class="item">
    <%= link_to image_tag(image_path("#{@title}/3.jpg")), home_path %>
  </div>

  <div class="item">
    <%= link_to image_tag(image_path("#{@title}/4.jpg")), home_path %>
  </div>

  <div class="item">
    <%= link_to image_tag(image_path("#{@title}/5.jpg")), home_path %>
  </div> 

  <div class="item">
    <%= link_to image_tag(image_path("#{@title}/6.jpg")), home_path %>
  </div>

  <div class="item">
    <%= link_to image_tag(image_path("#{@title}/7.jpg")), home_path %>
  </div>

  <div class="item">
    <%= link_to image_tag(image_path("#{@title}/8.jpg")), home_path %>
  </div>

  <div class="item">
    <%= link_to image_tag(image_path("#{@title}/9.jpg")), home_path %>
  </div>

</div>

I'm thinking you could use a for loop like this:

<% x = 1 %>

<% while x < 10 %>
  <%= link_to image_tag(image_path("#{@title}#{x}.jpg")), home_path %>
  <% x += 1 %>
<% end %>

Answer 1

You could pass an array of image paths into the view, defined like this:

@images = [1,2,3,4,5,6,7,8,9]

and then in the view, do this:

<% @images.each do |file_name| %>
  <%= link_to image_tag(image_path("#{@title}#{file_name}.jpg"), home_path %>
<% end %>

Defining your images in the controller means that the view only has to worry about rendering them. You could go one step further and actually generate the image path in the controller and pass that to the view instead, which IMO would be better:

# controller
images = [ 'myimg.jpg', 'myimg2.jpg' ]
# execute image_path helper method on each element of images
@paths = images.map {|name| image_path name }

# view 
<% @paths.each do |path| %>
  <%= link_to image_tag(path), home_path %>
<% end %>

I omitted the <div class="item"> tags because if it is just wrapping an img tag it is not necessary/semantic HTML. You could, by all means, add them back, in which case it would look like this:

# view 
<% @paths.each do |path| %>
  <div class="item">
    <%= link_to image_tag(path), home_path %>
  </div>
<% end %>

However, assuming that you're writing a carousel, and that each one of these images are actually grouped together, I would recommend something more along the lines of this

# view 
<div class="carousel">
  <% @paths.each do |path| %>
    <%= link_to image_tag(path), home_path %>
  <% end %>
</div >

You can then use the css selector .carousel img to target each image inside of the carousel (or alternatively .carousel a).

Answer 2

There's no need to create local variables in your view:

<% 1.upto(10) do |x| %>
  <%= link_to image_tag(image_path("#{@title}#{x}.jpg")), home_path %>
<% end %>