Unwanted behavior in a JavaScript function that uses Math.random in a for loop

Question

I was watching a YouTube tutorial about coding the 2048 game in Vanilla JavaScript, HTML and CSS. While I was watching the tutorial I thought I could also refactor the code given by the instructor. In a certain point, she calls twice a function to generate two numbered squares (not 0) in random places of the grid. I thought I could write a function (call it randomSquaresGenerator) that given the wanted amount of different positions could generate the respective amount of random numbered squares. Thing is that after some tests (refreshes on my browser) there are times where I can see fewer than the desired squares. I figured out that this is because in certain calls the function might check an already numbered square and passes. I still haven't figured on how to solve this though. Any ideas?

Here is my JavaScript code:

document.addEventListener("DOMContentLoaded", () => {
  const gridDisplay = document.querySelector(".grid");
  const scoreDisplay = document.querySelector(".score");
  const resultDisplay = document.querySelector(".result");
  const width = 4;
  const squaredWidth = width ** 2;
  let squares = [];

  function createBoard(width) {
    for (let i = 0; i < squaredWidth; i++) {
      square = document.createElement("div"); 
      square.innerHTML = 0; 
      gridDisplay.appendChild(square); 
      squares.push(square);
    }
  }
  
  //The function with the unwanted behavior.
  function randomSquaresGenerator(positions) {
    for (let i = 0; i < positions; i++) {
      let randomSquare = Math.floor(Math.random() * squaredWidth);
      if (squares[randomSquare].innerHTML == 0) {
        squares[randomSquare].innerHTML = 2;
      }
    }
  }

  createBoard();
  randomSquaresGenerator(2);
});

And my HTML code:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8" />
    <title>2048</title>
    <link rel="stylesheet" href="style.css" />
    <script src="app.js" charset="utf-8"></script>
  </head>
  <body>
    <div class="score-container">
      <div class="score-title">SCORE</div>
      <span class="score">1000</span>
    </div>
    <div class="grid"></div>
    <div class="result"></div>
  </body>
</html>

Don't use a for(...) with a fixed number of rounds. Use a while and count how many squares have been "generated". If the number is equal to positions then get out of the loop. — Andreas
– Andreas, Commented Jun 20, 2020 at 17:29
Thank you very much! That did work and now works for any given positions value. — akimo
– akimo, Commented Jun 20, 2020 at 17:50
Add your solution as an answer and accept it (, or delete the question) so it will be marked as "problem solved" — Andreas
– Andreas, Commented Jun 20, 2020 at 17:56

akimo · Accepted Answer · 2020-06-20 18:37:18Z

1

I followed @Andreas advice and used a while loop. It works fine. Here's the code:

function randomSquareGenerator(positions) {
    let generatedSquares = 0;
    while (generatedSquares != positions) {
      let randomSquare = Math.floor(Math.random() * squaredWidth);
      if (squares[randomSquare].innerHTML == 0) {
        squares[randomSquare].innerHTML = 2;
        generatedSquares += 1;
      }
    }
  }

answered Jun 20, 2020 at 18:37

akimo

234 bronze badges

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

Comments

Julio Suaya · Accepted Answer · 2020-06-20 18:10:26Z

0

I think the problem here is that in

let randomSquare = Math.floor(Math.random() * squaredWidth);

there you might be getting sometimes a position where you have already set. And in that case maybe you should try a different aproach.

function randomSquaresGenerator(positions) {
 let arrIndex=[];
 for(let i=0;i<squares.length;i++){
     arrIndex.push(i);
 }

 for (let i = 0; i < positions; i++) {
   let randomIndex = Math.floor(Math.random() * arrIndex.length);
   let randomSquare = arrIndex[randomIndex];
   arrIndex= [].concat(arrIndex.slice(0,randomIndex),arrIndex.slice(randomIndex+1,arrIndex.length));
   
     squares[randomSquare].innerHTML = 2;
   
 }

There I defined an array with the indexes of the main array, so , you just choose a random number from there, and it will map you to a position of the array that has a 0. Then you remove that element from the array and do the same, knowing that you will always get an available position. You may want to add an if statement to check if positions is bigger than the size of the array, just in case.

I hope you like it.

answered Jun 20, 2020 at 18:10

Julio Suaya

335 bronze badges

1 Comment

akimo Over a year ago

Thanks for your suggestion although I used the solution above. It's simpler and it works fine.

Collectives™ on Stack Overflow

Unwanted behavior in a JavaScript function that uses Math.random in a for loop

2 Answers 2

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related