7
\$\begingroup\$

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();
\$\endgroup\$
  • 2
    \$\begingroup\$ Can you should the HTML you want this to work on? \$\endgroup\$ – RoToRa Aug 28 '14 at 13:39
  • 1
    \$\begingroup\$ When does showPrev() come into play here? The HTML would definitely be helpful here so we can see how you're using the functions. Your code isn't bad at all, but I do see some opportunity for improvement. I just need a little more info. \$\endgroup\$ – Howard Renollet Nov 17 '14 at 21:51
2
\$\begingroup\$

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'), <- I would have called that slides since you are almost bound to select more than 1 slide with this selector
  • The repeated magical constants 3000 and 200 should be identified, named and centralized
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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