Making simple timer using javascript but getting weird behavior

Question

I'm a beginner at Javascript. I'm trying to make a timer. It works good, but when comes to the Pause and Stop buttons, they don't function well at all...

For Pause button.. let's say timer is on 0:58 when Pause button is pressed, it stops at 0:58 but when i press it again to resume the countdown .. it just become 0:57 .. And stops afterwards, it doesn't continue till 0:00 !
And for the Stop button.. When i simply press it, it gives this error :

Uncaught ReferenceError: checkstate is not defined

Here's the whole code combined so you guys can test it and let me know if there's something else :

<!DOCTYPE html>
<html>
    <head>
        <style>
            html, body, p, input, button {
                margin:0;
                padding:0;
            }

            html , body {
                height : 100%;
            }

            #mainContent {
                width : 300px;  
                height : 200px;
                margin: 50px auto;
            }

            #seconds {
                padding : 10px;
                font-size : 15px;
                font-weight:bold;
            }

            button {
                padding : 10px;
            }

            #interfereButton {
                margin : 0 40px;
            }


            #timer {        
                width :  200px;
                height: 100px;
                margin-top: 20px;
                font-weight:bold;
                font-size : 60px;
                text-align : center;
            }           
        </style>
        <title>Timer</title>        
    </head>

    <body>
            <div id="mainContent">
                <div id="userControl">
                    <input type="text" name="seconds" id="seconds"/>
                    <button id="start">Start</button>
                </div>
                <p id="timer">0:00</p>
                <div id="interfereButton">
                    <button id="pause">Pause</button>
                    <button id="stop">Stop</button>
                </div>
                <script>
                    var timer = document.getElementById("timer");
                    var start = document.getElementById("start");
                    var startPoint = document.getElementById("seconds");
                    var userControl = document.getElementById("userControl");
                    var userInterfere = document.getElementById("interfereButton");
                    var pause = document.getElementById("pause");
                    var stop = document.getElementById("stop");

                    var timerHandle;
                    var tempValue;

                    function checkState(){
                        if (timer.innerHTML == "0:00"){
                            userControl.style.display = "block";
                            userInterfere.style.display = "none";
                            timer.style.color = "black";
                        } else {
                            userControl.style.display = "none";
                            userInterfere.style.display = "block";
                        }
                    }

                    function activate(x){
                        var min = x.split(":")[0];
                        var sec = x.split(":")[1];

                        if(min >= 0){
                            sec--;
                            if(sec < 0){
                                sec = 59;
                                min--;
                                if(min < 0) {
                                    sec = "00" ;
                                    min = 0 ;               
                                    clearInterval(timerHandle);
                                }           
                            } else if(sec < 10) {
                                sec = "0" + sec;
                                timer.style.color = "red";          
                            }
                            timer.innerHTML = min + ":" + sec ;
                        } else {
                            clearInterval(timerHandle);
                        }   

                        checkState();
                    }


                    start.onclick = function() {
                        if(!isNaN(startPoint.value)){
                            timer.innerHTML= startPoint.value + ":00" ;
                            userControl.style.display = "none";
                            userInterfere.style.display = "block";
                            timerHandle = setInterval("activate(timer.innerHTML)" , 1000);
                        } else {
                            alert("Sorry, only numerical values are allowed.");
                        }
                    }

                    pause.onclick = function() {
                        if(pause.innerHTML == "Pause"){
                            tempValue = timer.innerHTML;
                            clearInterval(timerHandle);
                            pause.innerHTML = "Resume";
                        } else if(pause.innerHTML == "Resume"){
                            timerHandle = setInterval("activate(tempValue)" , 1000);
                            pause.innerHTML = "Pause";
                        }
                    }

                    stop.onclick = function(){
                        clearInterval(timerHandle);
                        timer.innerHTML = "0:00";
                        checkstate();   
                    }

                    window.onload = function(){
                        userInterfere.style.display = "none";
                    }   
                </script>
            </div>
    </body>

</html>

Correct me if I'm wrong, but should he wrap the JS in a function to prevent global variables? — Zach Lysobey
– Zach Lysobey, Commented Oct 5, 2011 at 15:13
@ZachL.: You are correct. It is a much better policy to explicitly declare a variable to be global than to allow all your outer-scope variables to be global by default. — benekastah
– benekastah, Commented Oct 5, 2011 at 15:24
@ZachL. Can you tell me how to do that ? is it just wrapping the code with function() { ... } ? — Rafael Adel
– Rafael Adel, Commented Oct 5, 2011 at 15:30
See the example in my answer: stackoverflow.com/questions/7663540/7663882#7663882 — Cheeso
– Cheeso, Commented Oct 5, 2011 at 15:44

James Montagne · Accepted Answer · 2011-10-05 15:22:21Z

4

Your problem is when you attempt to resume, you're calling set interval passing tempValue. tempValue is then not changed. So every time your interval fires, it's passing the same value to it. You can fix this by just eliminating tempValue and using timer.innerHTML the same way you do initially.

http://jsfiddle.net/WkuSG/

As for the stop button, check your method name, you've typoed, the S should be capitol.

Some other notes. You really shouldn't use setInterval passing a string. Internally that uses eval which is bad. It is better to pass a function. Something like this:

setInterval(function(){
    activate(timer.innerHTML);
}, 1000);

And finally, you can't really count on setInterval being accurate. If this timer needs to be very accurate you will need to compare timestamps. Since javascript is single-threaded, some other javascript could end up throwing off your time.

answered Oct 5, 2011 at 15:22

James Montagne

78.9k14 gold badges115 silver badges132 bronze badges

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

2 Comments

Rafael Adel Over a year ago

Oh, thanks that solved it. And thanks for the note. But i'm a beginner can you clarify the note about setInterval being not accurate ?

Cheeso Over a year ago

Re: accuracy of setInterval, see what John Resig has to say: ejohn.org/blog/how-javascript-timers-work

geekchic · Accepted Answer · 2011-10-05 15:21:44Z

1

I can answer part of your question: in your stop.onclick you're calling checkstate() whereas the function is called checkState. JavaScript is case sensitive.

answered Oct 5, 2011 at 15:21

geekchic

1,5661 gold badge11 silver badges21 bronze badges

Comments

Cheeso · Accepted Answer · 2011-10-05 15:46:39Z

1

First,

your call to checkstate() is wrong. It differs in case from the defined function, checkState.

B. I don't know what you're doing with the tempValue in the Pause/Result. Also there's no need to pass a value to your tick function (which you called "activate", but it's really a tick). Inside the tick function, just get the content of the DIV.

working example: http://jsbin.com/ohoqot

also, it's a good idea to wrap your functions in a scope:

(function(globalScope) {
    'use strict';

    var timer, start, startPoint, userControl, userInterfere, pause,
        stop, timerHandle, tempValue;

    function tick() { .... }

    ... other functions here ...

    window.onload = function(){
      setGlobals();
      setupBindings();
      userInterfere.style.display = "none";
    };

}(this));

edited Oct 5, 2011 at 15:46

answered Oct 5, 2011 at 15:36

Cheeso

193k106 gold badges486 silver badges734 bronze badges

1 Comment

Rafael Adel Over a year ago

Thanks for your effort. It really helped.

Collectives™ on Stack Overflow

Making simple timer using javascript but getting weird behavior

3 Answers 3

2 Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related