0

I have the following code in my index.html page

<body>
    <script type="text/javascript" src="words.js"></script>
    <script>
        var words = [];
        window.onload = function () {
            words = getwords();
        };
    </script>
</body>

And in word.js file

function getwords() {
    var block = [];
    var keyword = ['HELLO', 'CYCLE', 'APPLE', 'albatross', 'platform', 'OPERA', 'COURT', 'HOUSE', 'NEWEST', 'AEROPLANE', 'SCIENTIST', 'CORRIDOR', 'BUTTERFLY'.
        'MUSICAL', ' AUSTRALIA', 'XYLOPHONE', 'TAPESTRY', 'DREAM', 'NEEDLE', 'GIRAFFE'
    ];

    var index = [];
    for (var p = 0; p < keyword.length; p++) {
        index[p] = 0;
    }
    for (var i = 0; i < 8; i++) {
        var x = Math.floor(Math.random() * (keyword.length - 1));
        for (var j = 0; j <= i; j++) {
            if ((words[j] !== keyword[x]) && (index[x] !== 1)) {
                block[i] = keyword[x];
                index[x] = 1;
            }
        }
    }
    return block;
}

I want my getwords function to return any 8 words from keyword array everytime it is called in the onload and it should get stored in words array and those words shouldnt be repaeted next time. However my code doesn't work. May I know my mistake? Please help!

I tried

function getwords(){
var block = [], index =[];
var rem = keyword.length -1;
for(var p=0 ;p <(rem+1) ;p++)
{
index[p]=0;
}

 for(var i = 0; i < rem; ++i) keys.push(i);
 for(var i=0; i<8 ;i++) {
    var x = Math.floor(Math.random() * rem);
    while(index[x]===1)
    {
     x = parseInt(Math.random() * rem);
    }
    block.push(keyword[x]);
    index[x]=1;
    }

    return block;
    }

Still gives some same words on second call.

Kuhi
  • 25
  • 9
  • 1
    `for(var p=0 ;p< keyword.length ;p++)` low performance approach. – Alfred Huang Jun 07 '14 at 02:34
  • Specify what you mean by `my code doesn't work`, what isn't working? – Cyclonecode Jun 07 '14 at 02:36
  • When you say `those words shouldn't be repeated next time`, do you mean that if they reload the page, they shouldn't get the same list? – Ryan Erdmann Jun 07 '14 at 02:41
  • You got typos in your code. The `.` in the array should be `,` and `` should be ``. – Derek 朕會功夫 Jun 07 '14 at 02:47
  • calling array.length will iterates the array and have a O(array.length) linear time complexity. – Alfred Huang Jun 07 '14 at 02:57
  • and your random algorithm also takes O(8*N) complexity, not a good way, compare with my solution, your random taking procedure can be improved. – Alfred Huang Jun 07 '14 at 03:00
  • Oh, that doesn't matter at all in this case, I just want to notice that you can keep a better, not to compare with the length of an array in the for condition phrase. – Alfred Huang Jun 07 '14 at 03:04
  • Ok, thanks to notice that, I'm not sure about it now, but maybe the older web browser approach actually does this. And from some other materials I have read recommends to use `for(var i = arr.length-1; i >= 0; --i)` better than `for(var i = 0; i < arr.length; ++i)` – Alfred Huang Jun 07 '14 at 03:08
  • It is outdated, you are right, thank you. @user2864740 – Alfred Huang Jun 07 '14 at 03:09
  • Do you mean the words should not be repeated in the *returned set of words* or that the words should not be repeated *when the page is reloaded*? What happens if the page is reloaded several times, such that all the words are "used up"? – user2864740 Jun 07 '14 at 03:11
  • This code isn't giving any output by what I mean not working – Kuhi Jun 07 '14 at 04:45
  • I mean no single word should appear more than once at one reload. it can appear the next time but i.e all 8 words should be different at a time – Kuhi Jun 07 '14 at 04:46
  • it will be better if 8 words come at 1st time, next 8 later without repeat. – Kuhi Jun 07 '14 at 05:28

3 Answers3

1

A small mistake has cost you this problem ...

While storing the indexes in the index array you are using index[p] = 0;

But which should be 

for(var p = 0; p < keyword.length; p++) {
    index[p] = p;
}

here is the Working Example

Derek 朕會功夫
  • 92,235
  • 44
  • 185
  • 247
Runcorn
  • 5,144
  • 5
  • 34
  • 52
0

I can give you a nicer approach. Tested to be OK, have a try.

var keyword=[
    'HELLO', 'CYCLE', 'APPLE', 'albatross', 'platform',
    'OPERA', 'COURT', 'HOUSE', 'NEWEST', 'AEROPLANE',
    'SCIENTIST', 'CORRIDOR', 'BUTTERFLY', 'MUSICAL',
    'AUSTRALIA', 'XYLOPHONE', 'TAPESTRY', 'DREAM',
    'NEEDLE', 'GIRAFFE'];

var keys = [];
for(var i = 0; i < keyword.length; ++i) keys.push(i);

function getwords(count){
    var block = [];
    // first.
    // pick and remove [count] times. Becareful when keys goes empty.
    while(count > 0 && keys.length > 0) {
        var x = parseInt(Math.random() * keys.length);
        block.push(keyword[keys[x]]);
        keys[x] = keys[keys.length-1];
        keys.pop();
        --count;
    }
    return block;
}

console.log(getwords(8));
console.log(getwords(8));
console.log(getwords(8));
Alfred Huang
  • 17,654
  • 32
  • 118
  • 189
  • 1
    http://stackoverflow.com/questions/500504/why-is-using-for-in-with-array-iteration-such-a-bad-idea – Derek 朕會功夫 Jun 07 '14 at 02:50
  • @fish_ball .. ur solution worked. but the problem is I need the next set of 8 words when the function is called next. That is why I created the index array for marking/unmarking. i.e the js page should work as a datbase that always gives the next 8 words when called – Kuhi Jun 07 '14 at 05:33
  • That's simple, If you don't need it next time, just keep the keys array, I change the code and please check again. – Alfred Huang Jun 07 '14 at 05:44
-1

What you mean by "doesn't work"? What shows the console?

'BUTTERFLY'.'MUSICAL'. There's a dot instead of a comma.

Hope it helps.

Andre Medeiros
  • 224
  • 2
  • 6