Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Is there a better way to implement my carousel in less lines? Currently my solution is:

<div id="myCarousel" class="carousel slide">
  <div class="carousel-inner">
    <% @photos.each_with_index do |photo, index| %>
      <% if index == 0 %>
      <div class="item active">
        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">
        <div class="container">
          <div class="carousel-caption">
            <h1>TITLE</h1>
          </div>
        </div>
      </div>
      <% end %>
      <% if index == 1 %>
      <div class="item">
        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">
      </div>
      <% end %>
      <% if index == 2 %>
      <div class="item">
        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">
      </div>
      <% end %>
      <% if index == 3 %>
      <div class="item">
        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">
      </div>
      <% end %>
      <% if index == 4 %>
      <div class="item">
        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">
      </div>
      <% end %>
      <% if index == 5 %>
      <div class="item">
        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">
      </div>
      <% end %>
    <% end %>
  </div>
</div><!-- /.carousel -->
share|improve this question

3 Answers 3

up vote 4 down vote accepted

Get rid of unnecessary code:

<div id="myCarousel" class="carousel slide">
  <div class="carousel-inner">
    <% @photos.each_with_index do |photo, index| %>
      <% if index.zero? %>
      <div class="item active">
        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">
        <div class="container">
          <div class="carousel-caption">
            <h1>TITLE</h1>
          </div>
        </div>
      </div>
      <% else %>
      <div class="item">
        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">
      </div>
      <% end %>
    <% end %>
  </div>
</div>

Move stuff to helpers:

module PhotoHelpers
  def slider_image(photo)
    image_tag photo.image_url(:large), alt: photo.title
  end
end

<div id="myCarousel" class="carousel slide">
  <div class="carousel-inner">
    <% @photos.each_with_index do |photo, index| %>
      <% if index.zero? %>
        <div class="item active">
          <%= slider_image photo %>
          <div class="container">
            <div class="carousel-caption">
              <h1>TITLE</h1>
            </div>
          </div>
        </div>
      <% else %>
        <div class="item">
          <%= slider_image photo %>
        </div>
      <% end %>
    <% end %>
  </div>
</div>
share|improve this answer
    
Perfect - thank you. –  thmsmxwll Jan 21 '13 at 10:54
    
any reason for not indenting conditionals? –  tokland Jan 21 '13 at 12:18
    
@tokland no reason, it wasn't originally indented. I've updated. –  NARKOZ Jan 21 '13 at 12:31

Along with @NARKOZ's excellent answer, consder converting your template to haml:

#myCarousel.carousel.slide
  .carousel-inner
    - @photos.each_with_index do |photo, index|
      - if index.zero?
        .item.active
          %img{:alt => photo.title, :src => photo.image_url(:large)}/
          .container
            .carousel-caption
              %h1 TITLE
      - else
        .item
          %img{:alt => photo.title, :src => photo.image_url(:large)}/

All you have to do is name your template .haml instead of .erb, and include the haml gem in your Gemfile.

The haml gem comes with a conversion script, html2haml, which will convert your erb to haml.

share|improve this answer

Nothing serious, just take a look at this to learn how carousel control works.

private void CarouselGettingStarted_Load(object sender, EventArgs e)
{
    // create an elliptical path for the Carousel
    CarouselEllipsePath path = new CarouselEllipsePath();
    // Use the form center as the path center point
    path.Center = new WinFormsCarouselControl.UI.Point3D(45, 45, 0);
    path.FinalAngle = -90;
    path.InitialAngle = -80;
    path.U = new WinFormsCarouselControl.UI.Point3D(-15, -12, -45);
    path.V = new WinFormsCarouselControl.UI.Point3D(25, -20, -55);
    this.Carousel1.CarouselPath = path;
    // populate carousel items
    Carousel1.Items.Add(new ButtonElement("Solutions"));
    Carousel1.Items.Add(new ButtonElement("Services"));
    Carousel1.Items.Add(new ButtonElement("Support"));
    Carousel1.Items.Add(new ButtonElement("Contact"));
    LabelElement label = new LabelElement();
    label.Text = "Preferences";
    label.Font = new Font("Arial", 10, FontStyle.Italic | FontStyle.Bold);
    Carousel1.Items.Add(label);
}
share|improve this answer
3  
This looks like the code contains some valuable pointers, but on CodeReview it is expected that answers go in to more depth and cover more of the 'why' aspects. Why is this code more relevant/better than the HTML presented? What parts of the code relate to the issue the asker is habing? –  rolfl Dec 12 '13 at 11:39

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.