1
\$\begingroup\$

I have following code for a content scroller. This has been implemented with basic jquery. Iam trying to convert the same using advanced javascript concepts. Guide me to implement the same.

Working Code :

$(document).ready(function() {

var currentPosition = 0,
    slideWidth = 112,
    slides = $('#scroller ul li'),
    numberOfSlides = slides.length;
    scrollerContainer = $('#scroller ul');

$(scrollerContainer).css('width', slideWidth * numberOfSlides);

manageControls(currentPosition);

$('.control').click(function(){
    currentPosition = ($(this).attr('id')=='rightControl') ? currentPosition+1 : currentPosition-1;
    manageControls(currentPosition);
    $(scrollerContainer).animate({
        'marginLeft' : slideWidth*(-(currentPosition*3))
    });
});

function manageControls(position){
    if(position==0){ $('#leftControl').hide() }
    else{ $('#leftControl').show() }

    if((position*3)==numberOfSlides-1 || (position*3)==numberOfSlides-3 || (position*3)==numberOfSlides-2) { $('#rightControl').hide() }
    else{ $('#rightControl').show() }
}
});

Advanced Code Structure : Initial understanding

if(!scroller) { var scroller = {}; }

scroller =  
{
    videos : {

        moveThumbnails : function() {

        },
        gotonextThumbnails : function() {
            var obj = this;

        },
        gotopreviousThumbnails : function() {
            var obj = this;

        },
        init : function() {
            var obj = this;

        }
    },
    init : function() {
        var obj = this;
        this.videos.init();
    }
} 

Working example : http://jsfiddle.net/ylokesh/9EyEu/

\$\endgroup\$
  • 2
    \$\begingroup\$ I don't intend to troll, but do you have a definition of what's advanced and what's not in JavaScript? \$\endgroup\$ – Tharabas May 20 '12 at 18:34
  • \$\begingroup\$ @Tharabas at a beginner level, we used to write javascript where we marely use Object communication. Advanced javascript concpets like modular javascript so that later on same code can be converted to plugin easily. Just trying yo get my hands dirty with how object communicates and how we can achieve optimization. \$\endgroup\$ – Lokesh Yadav May 21 '12 at 17:08
4
\$\begingroup\$

I can't post a full answer about the "advanced JavaScript" part of your question, but here's what I would change in the actual code:

  • Use jQuery.fn.find method

    You could optimize this $('#scroller ul li') with this $('#scroller').find('ul li'). When using a selector like #someidname, jQuery can use directly the native method document.getElementById. When using #someidname something, then jQuery has to parse the selector, and only after this it can say "oh there's an id string in there !".

  • Don't recreate jQuery objects

    When doing var scrollerContainer = $('#scroller ul');, scrollerContainer becomes a jQuery object. Then, you don't need to do $(scrollerContainer).someMethodName() but you could directly write scrollerContainer.someMethodName().

  • Always use triple equals sign

    You should (almost) always prefer === over ==. The double equal sign may have some unexpected behavior such as "\t \n" == 0 which evaluate to true.

  • Don't use jQuery when it's not needed

    $(this).attr('id') could be simplified in this.id. This way, you don't construct a new jQuery object (this can be costly when done in loops for example), and the id property is supported in a cross-browser way so you don't have to worry about it.

  • Cache your DOM selections

    $('#leftControl') and $('#rightControl') should be cached in variables in the same way as $('#scroller ul') is. We should avoid doing the same DOM query wherever it's possible.

  • Prefer jQuery.fn.on or jQuery.fn.bind methods over traditional ones

    Because:

    • they are faster: internally $('whatever').click(fn) calls $('whatever').on('click', fn), so you can use it directly and avoid a function call,
    • they are more flexible: you can bind the same function to different events ($('whatever').on('click mouseenter mouseleave', fn)), you can namespace your events ($('whatever').on('click.myPlugin', fn)) which IMO would be useful in your case (your doing some sort of namespacing)
  • Simplify (Math) expressions

    IMO, the code slideWidth*(-(currentPosition*3)) is not really readable. Why using such parenthesis ? And what about this inner - sign ?

    What about writing it this way -slideWidth*currentPosition*3 ?

    Also, the part (position*3)==numberOfSlides-1 || (position*3)==numberOfSlides-3 || (position*3)==numberOfSlides-2 can be simplified. Have a look (shorter variable names for the example):

    (p*3)==n-1 || (p*3)==n-3 || (p*3)==n-2
    // which is equivalent to
    1==n-p*3 || 3==n-p*3 || 2==n-p*3
    // which is equivalent to
    1 <= n-p*3 && n-p*3 <= 3
    // and also equivalent to
    (n-p*3-1) in [0,0,0]
    

    so you end up with this:

    if( (numberOfSlides-position*3-1) in [0,0,0] ) {
        ...
    }
    

    That (n-p*3-1) in [0,0,0] part is quite interesting. When doing a in b you'll search if the array b has the index a. This only happens in our case because b is an array. So we use a array b = [0,0,0] which has 3 elements. 3 comes from the expression n-p*3, this is the maximum X of the three n-p*X forms seen above. Then, since [0,0,0] has 3 elements, its index are 0, 1, and 2. So the test if the expression n-p*3 equals to one of 1, 2, or 3, we have to subtract -1.

    To explain it on another way, we can say that 0 in [0,0,0], 1 in [0,0,0], and 2 in [0,0,0] will both be true (because we're using the array indexes and not its values), and all others numerical values will evaluate the expression to false. Since what you want you is to check if n-p*3 is of one 1, 2 or 3, if we remove 1 to this expression, we retrieve our values 0, 1, and 2 from above which will evaluate the expression to true.

    If you think this one is too tricky (and to be honest, it is !), you can do the following, which avoid code repetition and where the intent is explicit and readable (I would recommend this technique over the index i [0,0,0] one which is implicit and less readable IMO)

    var temp = numberOfSlides - position * 3;
    if( 1 <= temp && temp <= 3 ) {
        ...
    }
    

Hope that helps :)

\$\endgroup\$
  • \$\begingroup\$ Nice in on array of index trick. \$\endgroup\$ – Larry Battle May 20 '12 at 23:26
  • 2
    \$\begingroup\$ The in [0,0,0] "trick" is terribly unreadable. I wouldn't recommend it. \$\endgroup\$ – RoToRa May 21 '12 at 14:22
  • \$\begingroup\$ I'd support @RoToRa here. The trick will trick yourself after not having read that line of code for a while. Better extract the value and compare it in a more readable fashion like var v = numberOfSlides - position * 3 - 1; if (v >= 0 && v <= 2) ... as @pomeh recommended the line before the trick. \$\endgroup\$ – Tharabas May 21 '12 at 19:46
  • \$\begingroup\$ @RoToRa you're right. This is the kind of "high level" optimization, or the "art" to use JavaScript's leakiness to achieve a unexpected behavior. While I'm agree with you that this not readable and unclear (I've updated the answer to point it out), this could be the kind of trick used in "high level" optimization or "high level" of code compression, but that may represent a very low percentage or real-world applications ;) Anyway, I think this kind of JavaScript trick is funny, and that's why I've included it, but I would say don't use it unless you really know what're doing :) \$\endgroup\$ – pomeh May 21 '12 at 19:54
  • \$\begingroup\$ Maybe add use function binding instead of var that = this, etc? +1 for using .on() instead of .click(). \$\endgroup\$ – Cobby May 22 '12 at 6:28

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.