Random Array Repeats Index Items

Question

This code works fine in their example but repeats some of my index items when I try and use it.

var lastloaded = 0;
window.onload = loadPages;

    Array.prototype.knuthShuffle = function()
    {
        var i = this.length, j, temp;
        while ( --i )
        {
            j = Math.floor( Math.random() * (i - 1) );
            temp = this[i];
            this[i] = this[j];
            this[j] = temp;
        }
    };

var arr = ["aCard.html", "bCard.html", "cCard.html", "dCard.html"];

function loadPages () {
     arr.knuthShuffle();
    var frame = document.getElementById("frameWrap");
    if (lastloaded+1>arr.length){
        lastloaded = window.location = "greatJob.html";
    }
    frame.src = arr[lastloaded];
    lastloaded++;
};
document.getElementById('tom').onclick = loadPages;

Can anyone tell me what I am missing from this code to keep it from repeating items in my array?

When I try that code, I never get any repeated elements. I think that your assessment of what's going wrong may be incorrect, as the shuffle code looks fine. What is it that makes you think you're getting items repeated? — Pointy
– Pointy, Commented Oct 23, 2013 at 17:44
You mean that if you click arr.length times on tom that some pages are loaded multible times? If so that's because you shuffle in the loadPages (At every click on tom) — t.niese
– t.niese, Commented Oct 23, 2013 at 17:44
Where do you see repetitions in your array? If you mean the reloads, then please notice that "random" does not necessarily mean "changing" (see also dilbert.com/strips/comic/2001-10-25). — Bergi
– Bergi, Commented Oct 23, 2013 at 17:46
When I click on the 'tom' button to go to the next index it sometimes repeats the "bCard.html" or the "dCard.html". So 2 out of the 4 are repeats at some point. — Michael Sterling
– Michael Sterling, Commented Oct 23, 2013 at 17:48
If you throw a die multiple times, don't you sometimes get repeats? Why should this be any different? — Barmar
– Barmar, Commented Oct 23, 2013 at 17:55

Niet the Dark Absol · Accepted Answer · 2013-10-23 17:46:20Z

1

I'm not sure I completely understand how your page works, but it seems like it is shuffling the array in order to figure out the next page to go to. This means that it is newly shuffled with every page load, and you therefore have no guarantee as to the uniqueness of the pages - in fact, it would be extremely unlikely for you to get all the unique pages (1 in n!, to be precise)

In order to ensure uniqueness, you MUST save the generated order, not just the index you were at.

answered Oct 23, 2013 at 17:46

Niet the Dark Absol

326k86 gold badges480 silver badges604 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Michael Sterling Over a year ago

So, do I need to use the knuthShuffle to shuffle the array and store it in a new array and call that new array to use in the loadPages function? I am trying to send the user to the page and have it load up the first randomized item from the array and use the button to continue to go through the array until it ends and then it sends them to a completely different page. I tried to shuffle the original array into a new array but that didn't seem to work either. I am pretty new to this and this has got me baffled.

Niet the Dark Absol Over a year ago

Actually, looking closer at your code, just move the arr.knuthShuffle() line up one, just outisde the function loadPages definition. That way, you only shuffle the array once.

Community · Accepted Answer · 2017-05-23 12:21:09Z

~~You have a problem with your declaration of j and temp which might add up on multiple shuffles and give you some strange behavior.~~

specifically this line:

var i = this.length, j, temp;

and these lines:

j = Math.floor( Math.random() * (i - 1) ); temp = this[i];

The problem here is that you didn't actually declare the j and temp variables, that's an invalid syntax. Then when declaring them within your loop without the var keyword they are treated as global variables. Solvable by modifying the first line into:

var j, temp; var i = this.length;

Edit: Actually that isn't it, as already pointed out by t.niese every time you click tom you're re-shuffling.

What you want to do is shuffle once and use the newly shuffled array every time. So decouple your shuffling from your page loading by taking arr.knuthShuffle(); out of the loadPages() function.

Collectives™ on Stack Overflow

Random Array Repeats Index Items

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related