3
\$\begingroup\$

I'm at a loss as to how I could optimize this bit of code. I realize I could place these objects into some sort of array, but I'm not sure how to go about this.

$(document).ready(function () {
   var $stripe01 = $('.stripe-01');
   var $stripe02 = $('.stripe-02');  
   var $stripe03 = $('.stripe-03');  
   var $stripe04 = $('.stripe-04');  
   var $stripe05 = $('.stripe-05');    
   var $stripe06 = $('.stripe-06');      

$(window).scroll(function () {
    var s = $(this).scrollTop(),
        d = $(document).height(),
        c = $(this).height();

    scrollPercent = (s / (d - c) * 18);
    scrollPercent02 = (s / (d - c) * 14);      
    scrollPercent03 = (s / (d - c) * 30);            
    scrollPercent04 = (s / (d - c) * 4);            

    var position01 = -(scrollPercent * ($(document).width() - $stripe01.width()));
    var position02 = -(scrollPercent02 * ($(document).width() - $stripe01.width()));
    var position03 = -(scrollPercent03 * -($(document).width() - $stripe03.width()));      
    var position04 = (scrollPercent04 * -($(document).width() - $stripe04.width()));      

    $stripe01.css({
        'left': position01
    });
    $stripe02.css({
        'left': position02
    });
    $stripe03.css({
        'right': position03
    });      
    $stripe04.css({
        'right': position03
    });      
    $stripe05.css({
        'right': position03
    });      
    $stripe06.css({
        'left': position04
    });      
});
});
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Your question could be improved by including a short description of what the intended purpose of the code is. \$\endgroup\$ – 200_success Jul 3 '15 at 6:21
4
\$\begingroup\$

Basic simplifications

It's good to extract duplicated expressions to a local variable:

scrollPercent = (s / (d - c) * 18);
scrollPercent02 = (s / (d - c) * 14);      
scrollPercent03 = (s / (d - c) * 30);            
scrollPercent04 = (s / (d - c) * 4);

This is equivalent but shorter and safer:

var coef = s / (d - c);
scrollPercent01 = coef * 18;
scrollPercent02 = coef * 14;      
scrollPercent03 = coef * 30;            
scrollPercent04 = coef * 4;    

Similarly:

var position01 = -(scrollPercent * ($(document).width() - $stripe01.width()));
var position02 = -(scrollPercent02 * ($(document).width() - $stripe01.width()));
var position03 = -(scrollPercent03 * -($(document).width() - $stripe03.width()));      
var position04 = (scrollPercent04 * -($(document).width() - $stripe04.width()));

Extract the common $(document).width():

var width = $(document).width();
var position01 = -(scrollPercent * (width - $stripe01.width()));
var position02 = -(scrollPercent02 * (width - $stripe01.width()));
var position03 = -(scrollPercent03 * -(width - $stripe03.width()));      
var position04 = (scrollPercent04 * -(width - $stripe04.width()));      

Btw, the above statements look a bit confusing with the oddly placed negative signs. If I rearrange them, this should be equivalent:

var position01 = -(scrollPercent01 * (width - $stripe01.width()));
var position02 = -(scrollPercent02 * (width - $stripe01.width()));
var position03 =  (scrollPercent03 * (width - $stripe03.width()));      
var position04 = -(scrollPercent04 * (width - $stripe04.width()));      

Using arrays

To convert to using arrays to be able to process with a loop, you would need to create parallel arrays, each with the same number of elements. In this case 6, because that's the most diverse case you need to support. Something like this, but I'm not sure it's really any better or more readable than the original:

var $stripes = [
    $('.stripe-01'),
    $('.stripe-02'),
    $('.stripe-03'),
    $('.stripe-04'),
    $('.stripe-05'),
    $('.stripe-06')
];

$(window).scroll(function () {
    var s = $(this).scrollTop(),
        d = $(document).height(),
        c = $(this).height();

    var coef = s / (d - c);
    var scrollPercents = [
        coef * 18,
        coef * 14,
        coef * 30,
        coef * 4
    ];
    var positions = [
        - (scrollPercents[0] * (width - $stripes[0].width())),
        - (scrollPercents[1] * (width - $stripes[0].width())),
          (scrollPercents[2] * (width - $stripes[2].width())),
          (scrollPercents[2] * (width - $stripes[2].width())),
          (scrollPercents[2] * (width - $stripes[2].width())),
        - (scrollPercents[3] * (width - $stripes[3].width()))
    ];
    var layout_param = [
        'left',
        'left',
        'right',
        'right',
        'right',
        'left'
    ];

    for (var i = 0; i < $stripes.length; ++i) {
        $stripes[i].css({
            layout_param[i]: positions[i]
        });
    }
});
\$\endgroup\$
  • \$\begingroup\$ That was a perfect explanation! Thank you! I will be back to up-vote this when I get the reputation points. I really appreciate it. \$\endgroup\$ – unsider Jul 2 '15 at 21:39

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.