stilts77
stilts77

Reputation: 191

Variable not being written to array - Javascript

I am creating a simple 'bingo caller' tool. There will be a set of phrases in an array - I want to pick random phrases. However, once one is picked, I don't want it picked again.

At the moment - I'm just using random numbers (which will be used to pull the phrases from the array).

I have a function that generates the random number - based on the number of phrases in the function and then checks to see if it exists in an array of drawn numbers. If it doesn't - it is added if it does then the function stops (I want the function to run again - I was calling the function from within itself but it behaved erratically so I have simplified it to stop!).

The picked number is displayed as is the drawn number array too.

However, the function doesn't display how I'd expect it to: as soon as a duplicate number is arrived at, the code begins to add several elements to the array... I'm expecting it not to add anything if it's already there...

<!DOCTYPE HTML SYSTEM>
    <html>
    <head>
    <title>Teaching Styles Bingo</title>
    <script type="text/javascript">
    var words = [
    'One',
    'Two',
    'Three',
    'Four',
    'Five',
    'Six'
    ];
    var drawn=[];

    </script>   
    </head>
    <body>
    <button onclick="getNum()">Click me</button>
    <p id="never">Hello</p>
      <p id="collection"></p>
        <p id="first"></p>
        <script  type="text/javascript">
       // random number -> make it x
       // then, check is it the first number? If so - add to array
       // if not, then check: is it already in the array? If so - stop the function
       // if not, add it to the array and wait for further instruction
       // output to document the number picked and the contents of the array so far.
        function getNum()
        {
         var x=(Math.floor(Math.random() * (words.length)));
             var count=drawn.length;
            if (count<1)
                {
                drawn.push(x); 
                document.getElementById("never").innerHTML = x;  
                document.getElementById("collection").innerHTML = drawn;
                document.getElementById("first").innerHTML = "First Number";
                }
            else
        {
            for (var i=0;i<count;i++)
            {
               if (drawn[i] === x)
                   {
                       return;
                   }
                else
                {
                drawn.push(x); 
                document.getElementById("never").innerHTML = x;
                document.getElementById("collection").innerHTML = drawn;
                document.getElementById("first").innerHTML = "Not First Number";
                }
            }
        }   
       }
       </script>  
    </body>
    </html>

Why doesn't the script simply add nothing if the value exists in the array? Why does the script add several numbers to the array at a time (or at least - the output to html make it look like it does!)

Upvotes: 0

Views: 82

Answers (4)

Sylvan D Ash
Sylvan D Ash

Reputation: 1117

What you could try is when a phrase has been drawn, remove it from the original array and add it to the drawn array. This way, you don't even have to check if the phrase exists in the drawn array, since the original array will always contain phrases not drawn

var original = ['one', 'two', 'three', 'four', 'five', 'six'];
var drawn = []

function getIndex() {
  var randX = Math.floor(Math.random() * original.length;
  return randX;
}

var index = getIndex();
var drawnWord = original[ index ];
// Remove the word from original array
original.splice(index, 1);
// Add word to drawn array
drawn.push( drawnWord );

// Then proceed to display your words
// ...

Upvotes: 1

Chiu
Chiu

Reputation: 350

Because your drawn.push(x); is put inside the for loop.

Everytime drawn[i] === x fails, it pushes x to the end of drawn. Then your drawn will end up with multiple x. Move your drawn.push(x); out of your for loop to prevent it.

Even better, use indexOf() to check if x is already in drawn

if (drawn.indexOf(x) === -1) {
    drawn.push(x);
}

Upvotes: 2

Pugazh
Pugazh

Reputation: 9561

Something like this.

<!DOCTYPE HTML>
<html>

<head>
  <title>Teaching Styles Bingo</title>
  <script type="text/javascript">
    var words = [
      'One',
      'Two',
      'Three',
      'Four',
      'Five',
      'Six'
    ];
    var drawn = [];
  </script>
</head>

<body>
  <button onclick="getNum()">Click me</button>
  <p id="never">Hello</p>
  <p id="collection"></p>
  <p id="first"></p>
  <script type="text/javascript">
    // random number -> make it x
     // then, check is it the first number? If so - add to array
     // if not, then check: is it already in the array? If so - stop the function
     // if not, add it to the array and wait for further instruction
     // output to document the number picked and the contents of the array so far.
    function getNum() {
      var x = (Math.floor(Math.random() * (words.length)));
      console.log(x);

      var count = drawn.length;
      if (count < 1) {
        drawn.push(x);
        document.getElementById("never").innerHTML = x;
        document.getElementById("collection").innerHTML = drawn;
        document.getElementById("first").innerHTML = "First Number";
      } else if (drawn.indexOf(x) == -1) {
        drawn.push(x);
        document.getElementById("never").innerHTML = x;
        document.getElementById("collection").innerHTML = drawn;
        document.getElementById("first").innerHTML = "Not First Number";
      }
    }
  </script>
</body>

</html>

Upvotes: 2

ewcz
ewcz

Reputation: 13087

the problem is that it should first go through all the elements and check if x is there and push/ignore the element afterwards, i.e., to replace

        for (var i=0;i<count;i++)
        {
           if (drawn[i] === x)
               {
                   return;
               }
            else
            {
            drawn.push(x); 
            document.getElementById("never").innerHTML = x;
            document.getElementById("collection").innerHTML = drawn;
            document.getElementById("first").innerHTML = "Not First Number";
            }
        }

with something like

        var is_present = false;
        for (var i=0;i<count;i++)
        {
           if (drawn[i] === x)
               {
                   is_present=true;break;
               }
        }

            if(!is_present)
            {
            drawn.push(x); 
            document.getElementById("never").innerHTML = x;
            document.getElementById("collection").innerHTML = drawn;
            document.getElementById("first").innerHTML = "Not First Number";
            }

Upvotes: 1

Related Questions