current community

your communities

more stack exchange communities

Stack Exchange
tour help
careers 2.0
Take the 2-minute tour ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.
up vote 1 down vote favorite

I'm trying to write a simple Javascript(jQuery) function that randomly displays 6 Divs out of a possible 11. The code sort-of-works, it does randomly display around about half of the Divs, but it varaies between 4 and 8.

Can anyone tell me where I'm going wrong? It seems it should be so simple yet I'm completely lost!

My code:

<div class="offer">Offer 1</div>
<div class="offer">Offer 2</div>
... snip
<div class="offer">Offer 11</div>

<script src="query.min.js" type="text/javascript"></script>


 <script>
            var changed = 0;

            while (changed < 6) {


                $('.offer').each(function(index) {

                    if (changed < 6) {

                        var showOrNot = Math.floor(Math.random() * 2);

                        if (showOrNot == 1) {

                            $(this).addClass('offershow');
                            changed += 1;
                            $(this).text(changed); //debugging looking for the current value of changed 
                        }


                    }


                })


            }

        </script>
share|improve this question
    
Works pretty well when I test it - jsfiddle.net/infernalbadger/LX5xC It will occasionally do less than 6 because there's no check to make sure it doesn't change the same div twice. –  Richard Dalton Jun 6 '11 at 15:54
    
why are you outputting the divs in the first place, shouldn't this be a serverside/db job ? –  Val Jun 6 '11 at 15:55
    
Ah, that is what I hadn't accounted for. Thank you! Think Luke is probably right and a more 'exact' solution may be better, however. –  Jack Shepherd Jun 6 '11 at 15:57
    
ps Val: I'd very much prefer to do this server-side - and indeed my skills are more suited to that - but alas ...politics. –  Jack Shepherd Jun 6 '11 at 15:59

3 Answers 3

up vote 6 down vote accepted

The problem as it stands now is that you have a bunch of unrelated attempts. If you have a bucket with 11 balls and have a 50% chance to remove each ball, you could end up with any number of balls between 0 and 11. Probability is skewed toward the center, but you don't get six and exactly six each time.

What you want is to remove six, and exactly six, balls, selected arbitrarily.

Try something more like this:

var offers = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11];
for (var i = 0; i < 6; i += 1) {
    // choose a remaining offer at random
    var index = Math.floor(Math.random() * offers.length);

    // retrieve the item being shown
    var item = $('.offer').eq(offers[index]);
    item.addClass('offerShow');

    // remove this offer from the list of possibilities
    offers.splice(index, 1);
}

EDIT: In the comments, the OP clarified that what he really wants is to take an arbitrarily-sized list of offers and show six of them. The code provided below addresses that need, rather than the strict request in the original question. I'm leaving the original code for reference.

var OFFERS_TO_SHOW = 6;  // config, of sorts

// make sure no offers are shown now
$('.offer').removeClass('offerShow');

// show offers selected at random
for (var i = 0; i < OFFERS_TO_SHOW; i += 1) {
    // get a list of offers not already being shown
    var candidates = $('.offer').not('.offerShow');

    // select one from this list at random
    var index = Math.floor(Math.random() * offers.length);

    // show this offer by adding the offerShow class
    candidates.eq(index).addClass('.offerShow');
}
share|improve this answer
    
In the comments, I'm seeing a lot of discussion about the initial state before this code runs. I don't think that's the crux of your problem, but you can easily set initial state simply by removing the class from all offers at the outset: $('.offer').removeClass('offerShow'); –  Luke Sneeringer Jun 6 '11 at 15:59
    
I think this is certainly the way to go. Not too sure how to implement it with jQuery's each, but I'm sure I'll find a way. Thank you! –  Jack Shepherd Jun 6 '11 at 16:01
    
Will read and reply to your comment later this evening. Thanks again! –  Jack Shepherd Jun 6 '11 at 16:02
    
Why do you need jQuery's each at all? –  Luke Sneeringer Jun 6 '11 at 16:02
    
@Jack you won't be able to use each() since that's only to run the sequence exactly as many times as you have elements, not as many as you need. You can get your collection length with $('.offer').length and access the element directly with $('.offer')[i]. –  lincolnk Jun 6 '11 at 16:05
up vote 1 down vote

I think the problem is that you're not excluding divs that you've already set to show. So it's possible that your code to pick the next div to show is choosing one that's already been shown. If that makes sense? Trying changing your selector.....

$('.offer').not('.offershow').each(........

Keep in mind that addClass doesn't remove the existing classes, so your original selector will still prove true even though you've added the offershow class.

share|improve this answer
    
Thank you - this is what I'm going with. Luke did get there first in a comment hence the accepted answer... –  Jack Shepherd Jun 6 '11 at 17:15
    
No worries, he certainly gave a more detailed response and answer and you could've gone either way. :) Thanks for commenting though! –  WesleyJohnson Jun 7 '11 at 2:15
up vote 0 down vote

You're actually not checking wheter the selected div is already shown or not. Means when you're looping through all divs, there is always a possibility of setting a div visible twice. The script now thinks it was a new one and sets changed += 1;

Try adding something like this:

if(!$(this).hasClass('offershow')) {
    [[rest of code here]]
}
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.