Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

First question here so please be gentle. I have developed a jQuery slider and I want to implement it in WordPress (it will be a Featured posts slider).

This is my slider logic:

If there are 1 or 3 posts show them but hide prev and next button, if there are more then 3 posts but less or equal than 6 show navigation, and also if there are more than 6 posts show navigation and after animation is finished hide next button.

Now in my code there a lot if else statements so my question is: Is there a better way to write my slider?

You can look at the fiddle and try to delete div's from "container" (slider and its navigation will remain functional...)

My jQuery looks like this:

$(document).ready(function () {
  //If less than four divs Hide next button
  if ($('.container div').length < 4) {
  //Also hide prev button
  i = 0; //Click counter...
  $('.next').click(function () {
    //Prevent animation to mess up... 
    if ($('.container').is(':animated')) {
      return false;
    //Animate next click... 
      marginLeft: '-=300'
    }, 1000, function () {
      i++; //Increase Var by One...
      // Animation Callback If there are LESS than 6 divs 
      //hide next button         
      if ($('.container div').length <= 6) {
      // Else if there are more than 6 divs
      //show next button
      else if ($('.container div').length >= 6) {
      //If first click exists show prev button
      if (i == 1) {
      //If second click exists hide next button  
      else if (i == 2) {
  //on prev button click
  $('.prev').click(function () {
    //Prevent animation to mess up... 
    if ($('.container').is(':animated')) {
      return false;
    //Run animation...
      marginLeft: '+=300'
    }, 1000, function () {
      i--; //Decrase number of clicks
      //If on prev click counter===0 hide prev button
      //and show next button
      if (i == 0) {
      //Else if click===1 or click===0
      //Show next button         
      else if (i == 1 || i == 0) {

HTML looks like:

<div class="wrapper">
  <div class="container">//If you delete those guys below //Slider and navigation will still work!!
    <div class="one"></div>
    <div class="two"></div>
    <div class="three"></div>
    <div class="four"></div>
    <div class="five"></div>
    <div class="six"></div>
    <div class="seven"></div>
    <div class="eight"></div>
    <div class="nine"></div>
<div class="nav_wrapper">
  <div class="next">Next</div>
  <div class="prev">Prev</div>
share|improve this question
up vote 2 down vote accepted

Here's how I would have written the slider. For a small project like this, you probably don't need to use Prototypal Inheritance. However, if you wanted to scale or add new functionality it would be a walk in the park. As you dive into larger projects, taking time to plan how you're going to structure your code will greatly improve your ability to write the same. Keep in mind if any part of this is confusing or unclear, I'll be happy to walk you through it. I've included the code in this fiddle, but I'll do most of the explaining in the commented code below.

Now onto the fun stuff. You can place the following code in a separate .js file.

function Slider ( container, nav ) {
    //Get the elements passed in
    this.container = container;
    this.nav = nav;

    this.divs = this.container.find('div'); //Select the divs in the container
    this.divWidth = this.divs.width(); //Width of each div
    this.divsLen = this.divs.length; //Count how many divs there are

    this.current = 0; //Set the counter to the first div

//This function does all the animation
Slider.prototype.transition = function( speed ) {
        //Set the margin to a negative, hence the "-" in front.
        //Get the current div, multiply by its width, and multiply by 3.
        //This makes the animation move three divs at a time.
        //If you changed the width in the CSS, this code would work just fine.
        'margin-left': -( this.current * this.divWidth * 3 )
    }, speed); //Here I get the speed from the click event set up in your html file.

//Here is where it might get tricky. 
//This function controls the current set of divs being displayed.
Slider.prototype.setCurrent = function( dir ) {
    var pos = this.current; //Cache the current div

    //Okay. Take a breather right here :)
    //Get the current div and add to it depending on what is returned.
    //We test to see if the clicked button was 'next'.
    //If true, it returns '1' and adds that to 'pos'.
    //If false, that means you clicked 'prev'. It then returns a '-1' and adds to 'pos'.
    pos += ( ~~( dir === 'next' ) || -1 );

    //Now if we kept clicking 'prev', we would end up with a negative number after a while.
    //This tests to see if 'pos < 0'.
    //If yes, take the total divs, divide by 3, and subtract one.
    //This will make it display the last three divs. It kinda goes 'backwards'.
    //If pos isn't < 0, then take modulus of pos and the total / 3
    //Modulus is a math operator and if you're not clear on it, there's a link in the bottom.
    this.current = ( pos < 0 ) ? (this.divsLen / 3) - 1 : pos % (this.divsLen / 3);

    return pos;

This part comes after your script in the previous .js file. It can be in your html page at the bottom. I set up the previous piece of code so that we can refrain from referencing any DOM elements or specific values, keeping the logic separate. This part now sets all the configurations we and values to be sent to the slider.

//First off, if your user doesn't have JS on, your plugin will fail.
//I've set the wrapper overflow to scroll in the CSS, so if there is no JS, the
//user can still scroll and see all the divs.
//Here we set the overflow to hidden and select our container element.
var container = $('.wrapper').css('overflow', 'hidden').children('.container');

//Set up the Slider constructor function, and pass in the elements we want.
var slider = new Slider( container, $(".nav_wrapper") );

//Again, if there's no JS, there's no point in having buttons.
//The buttons are hidden in the CSS, and here we display then with .show().
//Then we select and set up the click event on the actual div button.'div').on('click', function() {
    //When clicked, we call the functions from our previous code, passing in our configurations.
    //By doing this, if we want to change anything, it can be done here and only done once.
    //Calls setCurrent and passes the clicked div class. It's either 'prev' or 'next'.
    slider.setCurrent( $(this).attr('class') );

    //After the current div is all figured out, we just animate.
    //Call transition and pass in the speed we want.
    slider.transition( 1000 );

Hopefully this all made sense to you. If you're a beginner in JS, this might be a little over your head, but that's perfectly ok! Try reading as many articles as you can find on the subjects here and you'll catch on soon.

Article explaining modulus:

share|improve this answer

Your Answer


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.