Issue with intervals and click functions

Question

I'm doing a project where we have to come up with a game where the square will change position based on math.random

I have two problems

1. it seems its running multiple instances of the function, as after a few cycles of the square repositioning - it will go bonkers and move all over the place in what seems like random intervals.

2. second problem is that i'm having a hard time getting the square div to registre the second time i click on it.

Link to JSFiddle : https://jsfiddle.net/ybpdjujb/

$(document).ready(function test() {
    $(document).on('click', '#target', function clicker() {
        $('#target').css('left', randPosX);
        $('#target').css('top', randPosY);
        clearInterval(fire)
        timeLeft = 2;
    });

    //Random
    var bodyWidth = $("#playArea").width();
    var bodyHeight = $("#playArea").height();
    var randPosX = Math.floor((Math.random() * bodyWidth / 1.1));
    var randPosY = Math.floor((Math.random() * bodyHeight / 1.1));

    //TIMER
    var timeLeft = 2;
    var elem = document.getElementById('time');
    var timerId = setInterval(countdown, 1000);

    function countdown() {
        if (timeLeft == 0) {
            doSomething();
            timeLeft = 5;
        } else {
            elem.innerHTML = timeLeft + ' seconds remaining';
            timeLeft--;
        }
    }

    function doSomething() {
        $('#target').css('left', randPosX);
        $('#target').css('top', randPosY);
        clearInterval(timerId);
    }

    var fire = setInterval(test, 2000);
});

Answer 1

TL;DR: The reason the square was going bonkers is that you were doing setInterval() in two places. One where you do var fire = setInterval(test, 2000); and one where you do var timerId = setInterval(countdown, 1000);. Remove one of these (and fix a few other things) and your code starts working.

But, we don't want to stop there, working code can soon become difficult to modify if not structured properly. So for your benefit and that of the programmer community let's see how we can refactor this code. I believe this will be useful for future coding (and debugging) exercises. If this is already common knowledge to you, consider this a refresher :)

Step 1: Put only what is required in document.ready

Here's a quick question. What do you want to do when the document is ready? Three things

1. Display the square
2. Setup the onclick handler
3. Start the timer

Let's keep only those and move the rest outside the document.ready block. One thing you have rightly done is comment regions of code you feel should go together, let's convert them into functions! While doing this we notice the stray setInterval() at the end of the file which we can remove. Your code should now look like this (refactoring that is done is explained below):

function doSomething() {
    positionSquareRandomly();
    clearInterval(timerId);
    timeLeft = 2;
    startTimer();
}

function positionSquareRandomly(){
    var bodyWidth = $("#playArea").width();
    var bodyHeight = $("#playArea").height();
    var randPosX = Math.floor((Math.random() * bodyWidth / 1.1));
    var randPosY = Math.floor((Math.random() * bodyHeight / 1.1));
    $('#target').css('left', randPosX);
    $('#target').css('top', randPosY);
}

function startTimer(){
    var timeLeft = 2;
    var elem = document.getElementById('time');
    var timerId = setInterval(countdown, 1000);
}

function countdown() {
    if (timeLeft == 0) {
        doSomething();
    } else {
        elem.innerHTML = timeLeft + ' seconds remaining';
        timeLeft--;
    }
}

function clicker() {
    doSomething();
}

$(document).ready(function test() {
    //Display square
    positionSquareRandomly();

    // Setup timer
    startTimer();

    // Setup click handler
    $(document).on('click', '#target', clicker);

});

Step 2: Refactor I

1. positionSquareRandomly() should get the co-ordinates and display the square. So we move the $('#target').css(..) calls to that method
2. Now when we do that clicker() and doSomething() look pretty similar, so clicker() now calls doSomething() which in turn positions the square and resets the timer i.e. clear the interval, reset timeLeft and call startTimer().
3. The clearInterval(fire) is gone as its not needed. timeLeft = 5 is also gone.

Step 3: Refactor II

Nice, but now we see that nothing happens! Press F12 to open the Developer console, Click on the Console tab. The error I see is Uncaught ReferenceError: timeLeft is not defined. This is because timeLeft is declared in startTimer() but we are trying to use it in countdown() too. We fix that by declaring timeLeft above all functions (making it global).

Similarly, put the var elem declaration at the top.

Declare var timerId = 0 at the top. The Id returned by setInterval() is always non-zero so this is a good initial value. (Set timeout example)

That's it!, add some useful elem.innerHTML in the if portion of countdown and we get this code.

var timeLeft = 2;
var elem = document.getElementById('time');
var timerId = 0;

function doSomething() {
    positionSquareRandomly();
    clearInterval(timerId);
    timeLeft = 2;
    startTimer();
}

function positionSquareRandomly(){
    var bodyWidth = $("#playArea").width();
    var bodyHeight = $("#playArea").height();
    var randPosX = Math.floor((Math.random() * bodyWidth / 1.1));
    var randPosY = Math.floor((Math.random() * bodyHeight / 1.1));
    $('#target').css('left', randPosX);
    $('#target').css('top', randPosY);
}

function startTimer(){
    timerId = setInterval(countdown, 1000);
}

function countdown() {
    if (timeLeft == 0) {
        elem.innerHTML = '0 seconds remaining';
        doSomething();
    }
    else {
        elem.innerHTML = timeLeft + ' seconds remaining';
        timeLeft--;
    }
}

function clicker() {
    doSomething();
}

$(document).ready(function test() {
    //Display square
    positionSquareRandomly();

    // Setup timer
    startTimer();

        // Setup click handler
    $(document).on('click', '#target', clicker);

});

There's a few things more we can do, but now I believe the code is clear, concise and above all, maintainable.