HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Simple slideshow with jQuery

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withsimplejqueryslideshow

Problem

I've made a very simple slideshow that works by appending and prepending images (on click and every 3 seconds). This is my first time trying to use classes or objects so any help would be great. I feel this could be a little verbose so I'd love any sort of feedback.

The user can click through the slideshow, but it also works on a timer of every 3 seconds.

// Scroll through images on the homepage

 function IndexSlideshow() {

    // declare index page html elements

    var contentSlideshow = $('.content-slideshow'),
       slide = $('.slide'),
       body = $('body'),
       self = this;

    // append the first slide, fade in the newly bumped slide 

    this.work = function() {
       $('.slide:first').fadeOut(200, function() {
          $(this).appendTo(contentSlideshow);
          $('.slide:first').hide().fadeIn(200);
       });
    }

    // show previous slide on click

    this.showPrev = function() {
       slide.click(function() {
          $(this).prependTo(contentSlideshow);
       });
    }

    // show next slide on click

    this.showNext = function() {
       slide.click(function() {
          self.work();
       });
    }

    // create an auto slide (slides every 3 seconds)

    this.auto = function() {
       var run;

       if (body.hasClass('home')) {
          run = setInterval(function() {
             self.work();
          }, 3000);

          // reset interval if they clicked an image

          slide.click(function() {
             clearInterval(run);
             run = setInterval(function() {
                self.work();
             }, 3000);
          });

       };

    }

 }

 var i = new IndexSlideshow();

 i.showNext();
 i.auto();

Solution

I like this code;

-
There are perhaps a few too many newlines, especially here:

});

      };

   }

}


  • work and auto are unfortunate function names, you could do better



  • slide = $('.slide'),



  • The repeated magical constants 3000 and 200 should be identified, named and centralized

Code Snippets

});

      };

   }


}

Context

StackExchange Code Review Q#61226, answer score: 2

Revisions (0)

No revisions yet.