DIV's are not moving using getElementsByClassName()

Viewed 64

I had started to create a game in HTML, CSS and JavaScript. I created a function that whenever I click on a button, all the red DIV's should start moving. However, I keep getting this error in the console:

Uncaught TypeError: Cannot read property 'style' of undefined. at index.html: 48

Due to this error, the DIV's don't move. I have used this code so far:

function startGame() {
  var gameObj = document.getElementsByClassName("object");
  for (var i = 0; i < gameObj.length; i++) {
    var h = setInterval(function() {
      gameObj[i].style.transition = "1s";
      gameObj[i].style.left = Math.floor(Math.random() * 90) + "%";
      gameObj[i].style.top = Math.floor(Math.random() * 90) + "%";
    }, 500);
    document.getElementById("stop").addEventListener("click", function() {
      clearInterval(h);
    });
  }
}
header {
  position: absolute;
  width: 100%;
  height: 10%;
  background-color: blue;
  top: 0;
  left: 0;
}

.object {
  width: 100px;
  height: 100px;
  border-radius: 50%;
  position: absolute;
  background-color: red;
  transition: 1s;
  top: 40%;
  left: 30%;
}

.butn {
  cursor: pointer;
  color: white;
  background-color: black;
  border: none;
  height: 50%;
  width: 5%;
}
<div class="object" id="object"></div>
<div class="object" style="top: 60%; left: 70%;"></div>
<header>
  <button onclick="startGame()" class="butn">Start</button>
  <button id="stop" class="butn">Stop</button>
</header>

When I used getElementById instead of getElementsByClassName, it worked. However, I can't use getElementById() since there are multiple DIV's. I am using the latest version of Chrome. Does anyone know that how can I fix it? Thanks in advance!

2 Answers

This is happening because you are using setInterval in the for loop.

There are two things happening here -

  1. setInterval being asynchronous in nature takes the end value of i during execution, which would be 2
  2. var being a function-scoped variable is not retaining the value inside block-scoped for loop

To solve this you can wrap the setInterval in a function and pass the value of i as an argument. Like so -

<html>

<head>
  <style>
    header {
      position: absolute;
      width: 100%;
      height: 10%;
      background-color: blue;
      top: 0;
      left: 0;
    }
    
    .object {
      width: 100px;
      height: 100px;
      border-radius: 50%;
      position: absolute;
      background-color: red;
      transition: 1s;
      top: 40%;
      left: 30%;
    }
    
    .butn {
      cursor: pointer;
      color: white;
      background-color: black;
      border: none;
      height: 50%;
      width: 5%;
    }
  </style>
</head>

<body>
  <div class="object" id="object"></div>
  <div class="object" style="top: 60%; left: 70%;"></div>
  <header>
    <button onclick="startGame()" class="butn">Start</button>
    <button id="stop" class="butn">Stop</button>
  </header>
  <script>
  function moveElem(gameObj, i) {
    var h = setInterval(function() {
       gameObj[i].style.transition = "1s";
       gameObj[i].style.left = Math.floor(Math.random() * 90) + "%";
       gameObj[i].style.top = Math.floor(Math.random() * 90) + "%";
    }, 500);
    document.getElementById("stop").addEventListener("click", function() {
       clearInterval(h);
    });
  }

  function startGame() {
    var gameObj = document.getElementsByClassName("object");
    for (var i = 0; i < gameObj.length; i++) {
      moveElem(gameObj, i);
    }
  }
  </script>
</body>

</html>

This is a classic mistake you are bound to make when you are learning Javascript. The following articles will help you get a better understanding -

  1. Closure - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Closures
  2. Asynchronous - https://developer.mozilla.org/en-US/docs/Learn/JavaScript/Asynchronous

Move setInterval() at the root level of the startGame()

function startGame() {
  var h = setInterval(function() {
    var gameObj = document.getElementsByClassName("object");
    for (var i = 0; i < gameObj.length; i++) {
      gameObj[i].style.transition = "1s";
      gameObj[i].style.left = Math.floor(Math.random() * 90) + "%";
      gameObj[i].style.top = Math.floor(Math.random() * 90) + "%";
    }
  }, 500);

  document.getElementById("stop").addEventListener("click", function() {
    clearInterval(h);
  });
}
header {
  position: absolute;
  width: 100%;
  height: 10%;
  background-color: blue;
  top: 0;
  left: 0;
}

.object {
  width: 100px;
  height: 100px;
  border-radius: 50%;
  position: absolute;
  background-color: red;
  transition: 1s;
  top: 40%;
  left: 30%;
}

.butn {
  cursor: pointer;
  color: white;
  background-color: black;
  border: none;
  height: 50%;
  width: 5%;
}
<div class="object" id="object"></div>
<div class="object" style="top: 60%; left: 70%;"></div>
<header>
  <button onclick="startGame()" class="butn">Start</button>
  <button id="stop" class="butn">Stop</button>
</header>

Related