current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite
1

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

1 Answer 1

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
1  
Guess I don't need to add an answer now .... +1, and welcome to CR –  rolfl Feb 28 '14 at 3:49
    
@rolfl: Thanks. You could still add a loop answer since that's what is specifically being asked. If the ranges aren't so tidy, then that would be needed. –  cookie monster Feb 28 '14 at 3:49
    
I was going to add essentially the exact same answer as you (why use a loop when math can do the work for you). –  rolfl Feb 28 '14 at 3:50
2  
wow, thank you. I was so stuck on thinking I needed a loop, but this works very well. –  Randall987 Feb 28 '14 at 3:51
2  
Welcome to CR! Feel free to come join us in the chat room sometime! :) –  syb0rg Feb 28 '14 at 3:54

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.