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
 }); 
});
});
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 2, 2015 at 19:36
\$\endgroup\$
1
  • \$\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\$ Commented Jul 3, 2015 at 6:21

1 Answer 1

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]
 });
 }
});
answered Jul 2, 2015 at 21:04
\$\endgroup\$
1
  • \$\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\$ Commented Jul 2, 2015 at 21:39

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.