When i don't use hitPositoin = null

Hi, I’m learning whac-a-mole game code and I have a question at the part I pointed.
when I don’t use (hitPosition = null) the result I get is equal to the number of time function (randomSquare()) is executed.
Why do this happening?

        const squares = document.querySelectorAll('.square');
        const mole = document.querySelector('.mole');
        const timeLeft = document.querySelector('#time-left');
        const score = document.querySelector('#score');

        let result = 0;        
        let hitPosition = null;
        function randomSquare(){

            squares.forEach(square => {
                square.classList.remove('mole')
            })

            let randomSquare = squares[Math.round(Math.random() * 9)]
            randomSquare.classList.add('mole');
            hitPosition = randomSquare.id;
           
 //  ????? I have question at this part ??????
            squares.forEach(square => {
                square.addEventListener('mousedown', () => {
                    if(square.id == hitPosition){
                        result++;
                        score.textContent = result;
                        hitPosition = null;
                        
                    }

            })}
            )
            
        }
        function moveMole(){
            let timerId = null;
            timerId = setInterval(randomSquare, 1000)
        }
        moveMole();
        
       
        

It is because of the way you added the ‘mousedown’ listeners. Every second your app runs, you are adding a mousedown event listener to each square. That means by the time your app runs 1 minute, you have already added 60 even listeners. Each time you hit the mole, you are actually incrementing result by the number of event listeners already attach to that square.

To remedy this problem, you should move the code that creates the mousedown event listeners to be outside of your randomSquare function. That way you only ever attach one mousedown event listener for each square. You can then safely remove hitPosition = null from the callback function.

Another thing you can do to improve the efficiency of the app, is since you are already keeping track of the latest value of hitPosition, why not only remove the class named mole from the current square? There is no sense in iterating through all the squares when you know which one has the mole in it.

The above method in your code will sometimes produce a number larger than a valid index of the squares node list. I suggest using Math.floor instead.

1 Like

Thank you very much. :pray: :pray: :pray:

Were you able to make any of the changes I suggested?

Yes i did but I stuck how to remove class mole from current square

Post your current code and I can guide you to a solution. It would nice if you posted your HTML and CSS, so I can see what the game actually looks like.

html

<h2>Your Score: </h2>
        <h2 id="score">0</h2>

        <h2>Time Left: </h2>
        <h2 id="time-left">60</h2>

        <div class="grid">
            <div class="square" id="1"></div>
            <div class="square" id="2"></div>
            <div class="square" id="3"></div>
            <div class="square" id="4"></div>
            <div class="square" id="5"></div>
            <div class="square" id="6"></div>
            <div class="square" id="7"></div>
            <div class="square" id="8"></div>
            <div class="square" id="9"></div>
        </div>

css

*{
            box-sizing: border-box;
        }
        
        .grid {
            width: 600px; 
            height: 600px;
            display: flex;
            flex-wrap: wrap;
        }
        .square {
            width: 200px;
            height: 200px;
            border: 2px solid green;
        }
        .mole {
            background-color: blue;
        }

js

        const squares = document.querySelectorAll('.square');
        const mole = document.querySelector('.mole');
        const timeLeft = document.querySelector('#time-left');
        const score = document.querySelector('#score');

        let result = 0;        
        let timerId = null;
        let hitPosition = null;
        let currentTime = 60;
        // random square 
        function randomSquare(){

            squares.forEach(square => {
                square.classList.remove('mole')
            })

            let randomSquare = squares[Math.floor(Math.random() * 9)]
            randomSquare.classList.add('mole');
            hitPosition = randomSquare.id;
    
        }   
        // player 
        squares.forEach(square => {
                square.addEventListener('mousedown', () => {
                    
                    if(square.id == hitPosition){
                        result++;
                        score.textContent = result;     
                    }

            })}
            )
            // every second run randomSquare
        function moveMole(){
 
            timerId = setInterval(randomSquare, 1000)
        }
       
        moveMole();
        // time left
        function countDown(){
            currentTime--;
            timeLeft.textContent = currentTime;
            if(currentTime == 0){
                clearInterval(counterTimeId)
                clearInterval(timerId)
                alert('Game Over and your score is: ' + result)
            }


        }
        let counterTimeId = setInterval(countDown , 500)

I changed the HTML so the ids started at 0

<h2>Your Score: </h2>
<h2 id="score">0</h2>

<h2>Time Left: </h2>
<h2 id="time-left">60</h2>

<div class="grid">
  <div class="square" id="0"></div>
  <div class="square" id="1"></div>
  <div class="square" id="2"></div>
  <div class="square" id="3"></div>
  <div class="square" id="4"></div>
  <div class="square" id="5"></div>
  <div class="square" id="6"></div>
  <div class="square" id="7"></div>
  <div class="square" id="8"></div>
</div>

Then, I was able to write the following. Notice how I renamed the variables to make the code more readable.

function moveMole() {
  timerId = null;
  squares[molePosition].classList.remove("mole");
  const randomSquare = squares[Math.floor(Math.random() * squares.length)];
  randomSquare.classList.add("mole");
  molePosition = randomSquare.id;
  timerId = setTimeout(moveMole, 1000);
}

function countDown() {
  currentTimeLeft--;
  timeLeft.textContent = currentTimeLeft;
  if (currentTimeLeft == 0) {
    clearInterval(counterTimeId);
    clearInterval(timerId);
    alert("Game Over and your score is: " + result);
  }
}

const squares = document.querySelectorAll(".square");
const timeLeft = document.querySelector("#time-left");
const score = document.querySelector("#score");

squares.forEach((square) => {
  square.addEventListener("mousedown", () => {
    if (square.id == molePosition) {
      score.textContent = ++result;
    }
  });
});

let result = 0;
let molePosition = 0;
let currentTimeLeft = 60;
let counterTimeId = setInterval(countDown, 1000);
let timerId = null;
moveMole();

To make this game more realistic, see if you can make the mole movement random (i.e somtimes it shows up in another cell in 1 sec and the next time it is 1/2 a second).

Also, you can cheat at the game by clicking on the mole square really fast before it moves to the next random square. Ideally, you would want to remove the mole class and disable the ability to click on that cell until the next mole move to prevent someone from clicking on it after that hit position has already been clicked.

1 Like

I did some changes in moveMole function, I made the mole movement random,
in the next movement it will not show in the same cell

function moveMole(){
            timerId = null;
            squares[molePosition].classList.remove('mole')
            let randomSquare = squares[Math.floor(Math.random() * squares.length)];

            // to prevent mole to show in the same cell
            if(randomSquare.id == molePosition){
               while (randomSquare.id == molePosition){
                    randomSquare = squares[Math.floor(Math.random() * squares.length)]
                    }
            }
            
            randomSquare.classList.add('mole')
            molePosition = randomSquare.id;
            timerId = setTimeout(moveMole, 650);
        }

and another thing I want to disable EventListener() when time is over
because I can click and get the point
I used removeEventListener() for each square but it doesn’t work
How can I solve this?

Can you post your latest code or better yet put all the html, css, and js into Codepen?

You have two issues:

  1. You need to review the syntax of removeEventListener. You must pass two arguments to it.
  2. You will need to rethink when to add the mousedown event listener and when to remove it. Currently, you add one to each square, but then you attempt to remove all of them (squares.forEach. Once the event listener is removed, other squares that have the mole will not change the score because there is no longer a mousedown eventlistener attached.

My suggestion is to only attach an event listener when a specific square has been chosen to be the mole and then remove just that square’s even listener when the mole has been clicked (and it is still a mole).