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

I'm working on extending the functionality of a photo gallery by adding a thumbnail display, I have multiple if statements set up to adjust the position of the thumbnails. Obviously this is not ideal, I'm trying to put my head around how to use a for loop to iterate the width multiplier and curSlide position.

Ideally I would like the curSlide to increment after every 5 slide and the thumbGalwidth*i to increment as well.

var thumbGalwidth = 795;

if(currSlide < 6) {
jQuery('#view-all-container').css('margin-left', '-' + thumbGalwidth*0 + 'px');
}
if(currSlide >= 6) {
jQuery('#view-all-container').css('margin-left', '-' + thumbGalwidth*1 + 'px');
}
if (currSlide >= 11) {
jQuery('#view-all-container').css('margin-left', '-' + thumbGalwidth*2 + 'px');
}
if (currSlide >= 16) {
jQuery('#view-all-container').css('margin-left', '-' + thumbGalwidth*3 +     'px');
}
if (currSlide >= 21) {
jQuery('#view-all-container').css('margin-left', '-' + thumbGalwidth*4 +    'px');
}
if (currSlide >= 26) {
jQuery('#view-all-container').css('margin-left', '-' + thumbGalwidth*5 +    'px');
}
share|improve this question
    
How does curSlide change over time in this code? It looks quite fixed, and I suspect you omitted the looping code calling this function. Please provide enough context to advise you intelligently. – David Harkness Feb 28 '14 at 3:51
    
Be wary with accepting answers so quickly. It could discourage future reviews from being written. – syb0rg Feb 28 '14 at 3:56
up vote 5 down vote accepted

Because you have nice, predictable ranges, you don't need a loop at all. Just do a little math.

var thumbGalwidth = 795;

var n = Math.floor(((currSlide || 1)-1) / 5);

jQuery('#view-all-container').css('margin-left', '-' + thumbGalwidth*n + 'px');

If the largest n should be 5, then change the assignment to this:

var n = Math.min(5, Math.floor(((currSlide || 1)-1) / 5));
share|improve this answer

Your Answer

 
discard

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.