FlyingUnderpants
FlyingUnderpants

Reputation: 101

Making javascript / jquery code shorter?

Well I'm kind of new with jQuery and javascript and I have written 'some' code for a image slider but it is enormous. I am convinced it can be shorter, but I don't know how. Could somebody show me how it sould be done so I can learn from it ? Thanks in advance :D

PS I don't want to look lazy, I really want to get better in Javascript and Jquery

Here is my code

jQuery(document).ready(function(){

//Variables for image slider
var imgWrapper = $('.carousel-wrapper'),
    img = $('.carousel-box'),
    imgWidth = $('.carousel-box').width(), 
    imgLength = $('.carousel-box').length, // 4
    currentMargin = imgWrapper.css('margin-left').replace('px',''), //0px
    responsiveLength = 3,
    maxMargin = -((imgLength - responsiveLength) * imgWidth), 
    minMargin = 0; //0px

$(window).resize(function(){
    if ($(window).width() <= 1080) {
        responsiveLength = 2;
        maxMargin = -((imgLength - responsiveLength) * imgWidth);
    } else if ($(window).width() <= 700) {
        responsiveLength = 1;
        maxMargin = -((imgLength - responsiveLength) * imgWidth);
    } else {
        responsiveLength = 3;
        maxMargin = -((imgLength - responsiveLength) * imgWidth);    
    }
});

//Transition animation on click
$('.portbutton').click(function(){

    //Get the direction
    var dir = $(this).data('dir');

    if (dir === 'next' && currentMargin != maxMargin) {
        currentMargin -= imgWidth;
        imgWrapper.animate({marginLeft: currentMargin},300);
    } else if (dir === 'next' && currentMargin === maxMargin){
        currentMargin = minMargin;
        imgWrapper.animate({marginLeft: currentMargin},300);
    } else if (dir === 'prev' && currentMargin != minMargin){
        currentMargin += imgWidth;
        imgWrapper.animate({marginLeft: currentMargin},300);
    } else {
        currentMargin = maxMargin;
        imgWrapper.animate({marginLeft: currentMargin},300);
    }

});

});

Upvotes: 0

Views: 44

Answers (2)

TheEllis
TheEllis

Reputation: 1736

You code isn't really that bad. One thing to know about javascript, and especially using jquery, it does get / look messy and verbose. There have been attempts to clean up javascript syntax with CoffeeScript, but it hasn't caught on in mainstream development as much as some had hoped (myself included).

Also, making things as abbreviated as possible is not always in your's and other's best interest. You might end up making your code a whole lot less readable and therefore less maintainable when you come back to change the code a year later.

That being said, you could perhaps make the code cleaner and more readable by splitting some of your logic into separate functions. Anonymous functions are nice (and more or less standard), but you could perhaps break those out into named functions.

Upvotes: 1

Sergi Case
Sergi Case

Reputation: 226

You can use some functions for:

function animate(currentMargin){
 imgWrapper.animate({marginLeft: currentMargin},300);
}

function getMaxMargin(imgLength,responsiveLength,imgWidth){
 maxMargin = -((imgLength - responsiveLength) * imgWidth);
 return maxMargin  
} 

And maybe avoid create all time the value responsiveLength, if it just a integer that you always set before use , then just punt the integer instead, you will have more memory space.

Upvotes: 2

Related Questions