0

I was recently making a timer object and a ticking function that would tick every second according to a setTimeout loop. However, there was no delay in the ticking. If you try out the below code, you will find that the time number has increased to the thousands in only a few seconds. What, if anything, am I doing wrong?

<html>

<head>

</head>
    <button onclick="startTimer()">Start</button>
    <button onclick="stopTimer()">Stop Timer</button>
    <button onclick="readTimer()">Read Timer</button>

    <script>
function tick(){
    console.log("TICK TOCK");
    if(this.running == true){
        this.time += 1;
        console.log("HELLO!");
        setTimeout(this.tick(), 1000, $(this));
    }

}

function start(){
    this.running = true;
    this.tick();
}
function read(){
    return this.time;
}
function stop(){
    this.running = false;
}

function reset(){
    if(this.running == false){
        this.time = 0;
    }else {
        this.time = 0;
    } 
    this.tick();
}
function timer(){
    this.running = false;
    this.time = 0;
    this.start = start;
    this.read = read;
    this.stop = stop;
    this.reset = reset;
    this.tick = tick;
}
var t = new timer();

        function startTimer(){
            t.start();
        }   
        function stopTimer(){
            t.stop();
        }
        function readTimer(){
            alert("This is the current Timer Reading: " + t.time);
        }





    </script>
</html>
TGrif
  • 5,725
  • 9
  • 31
  • 52
SweetFeet
  • 132
  • 1
  • 1
  • 14
  • 1
    You are passing an executed function, instead of a function to `setTimeout`. Do `setTimeout(this.tick.bind(this), 1000)`. – trincot May 30 '17 at 22:53
  • It works! Thanks very much. I would not have solved this problem without your help. – SweetFeet May 30 '17 at 23:29

3 Answers3

1

Your error is that you call setTimeOut on this.tick(). When you call this inside the tick() function, you are already referring to the tick function, so you want to use setTimeout(this, 1000); and your timer will work properly.

See this fiddle for solution: https://jsfiddle.net/sg7yf6r4/

Read more about the issue: Javascript objects calling function from itself

Felix Guo
  • 2,700
  • 14
  • 20
  • For some reason, changing the code from setTimeout(this.tick(), 1000); to setTimeout(this, 1000) produces an error: Uncaught SyntaxError: Unexpected identifier. – SweetFeet May 30 '17 at 23:30
  • Double check your code; here is a CodePen that works as intended: https://codepen.io/anon/pen/LyKwXG Make sure you haven't spelt anything wrong. – Felix Guo May 31 '17 at 01:38
  • I checked my spelling and couldn't find any reason for it not to work, and yet it still only runs the tick function one time before leaving an error message. function tick(){ console.log("TICK TOCK"); if(this.running == true){ this.time += 1; console.log("HELLO!"); setTimeout(this, 1000); } } – SweetFeet May 31 '17 at 01:48
  • Which line is the SyntaxError occurring on? Can you post that line specifically? – Felix Guo May 31 '17 at 01:50
  • Right. The console log in chrome does not give a line in the html file but gives this: " VM80:1 Uncaught SyntaxError: Unexpected identifier ." That probably isn't helpful is it. Actually, I copied the code at CodePen into my html file and it still makes the same error. Maybe my chrome browser is messed up. – SweetFeet May 31 '17 at 01:53
  • I think Chrome doesn't like the passing of `this` as a function in that context. You could try replacing `this` with `this.tick` and see if Javascript can resolve that properly. That also works on CodePen. – Felix Guo May 31 '17 at 01:58
  • Actually, the code seems to be making the same error in code pen. Did you by any chance see the time increment any higher than one without pressing the start button again? It doesn't for me, most likely because the tick function runs once and increments the time by one, but then fails to do the set-timeout, which is necessary for an artificial loop. – SweetFeet May 31 '17 at 02:05
  • You're right, for some reason, JS isn't scoping the variables properly. Here is a JS fiddle that works for sure, https://jsfiddle.net/sg7yf6r4/, and here is some information about how I fixed it https://stackoverflow.com/questions/16841322/javascript-objects-calling-function-from-itself. – Felix Guo May 31 '17 at 03:25
  • Thanks! It works now! I had no idea that the issue wasn't a simple typo, but rather a complex problem where the function being passed to setTimeout was already executed. Thanks! – SweetFeet May 31 '17 at 10:57
  • If it helped please mark the answer as accepted so future visitors know your problem is solved :) – Felix Guo May 31 '17 at 17:19
  • K. But, how do I mark multiple answers? Your answer in the comments work, but trincot's answer works as well. – SweetFeet May 31 '17 at 19:29
  • You can only choose one accepted answer, so you can pick whichever one you thought solved your problem in the best way – Felix Guo May 31 '17 at 19:44
1

The first parameter of setTimeout should be a function. However, you are passing it the result of a function. So instead of setTimeout(this.tick(), 1000, $(this)); you should use setTimeout(this.tick, 1000, $(this));.

Wayne Allen
  • 1,605
  • 1
  • 10
  • 16
1

You are passing an executed function, instead of a function reference to setTimeout. To pass the function itself, remove the parentheses. Secondly, to make sure this will still be the current this when tick is eventually invoked, use .bind(this).

Note that the third argument of setTimeout would pass that value to tick, which is of no use in your case. NB: That $(this) is probably a remnant of some other code, since the $ would be typically used with jQuery, which you are not using.

So taking that all together, do:

setTimeout(this.tick.bind(this), 1000)
trincot
  • 317,000
  • 35
  • 244
  • 286