CODEWITHSUNDEEP
javascript
monthon1
monthon1

Reputation: 245

countdown in javascript

I have got a piece of code, that should countdown some number (in this example 1111) and write it into span id="timeframe". But it doesn't work, i dont know why. Can you tell me where is my mistake, please? 

<script language="JavaScript">
  <!--
  var txt_minute = "Min";
  var txt_second = "s";
  var time_left = 1111;
    var temp=timeleft;
              houres = Math.floor(temp / 3600); 
              temp %= 3600;
              minutes = Math.floor(temp / 60)";
              temp %= 60;
              seconds= temp;

function display()
{
    time_left -= 1;
    if (seconds <= 0) {
        seconds = 59;
        minutes -= 1;
    } else {
        seconds -= 1;
    }
if (minutes <= 0){
minutes=59;
houres-=1;
}

    if (time_left <= 0) {
        seconds = 0;
    }



    if (time_left > 0) {
        document.getElementById("timeframe").innerHTML = houres +" hours" + minutes + " " + txt_minute + " " + seconds + " " + txt_second;
    } else {
        window.location.reload();
    }
    setTimeout("display()",1000);
}

display();

//-->
</script>

Upvotes: 0

Views: 729

Answers (5)

Wim
Wim

Reputation: 11252

As an additional improvement (but first get the basic version working by reading everyone else's comments), I like to replace this line

   setTimeout(display, 1000);

with this:

   var next = 1010 - (new Date()).getTime() % 1000;
   setTimeout(display, next);

The first version will never run exactly every second (due to other processes running and other inaccuracies in setTimeout). This can make your display jump 2 seconds at once when one call of display() happens at the end of second n and the next call only happens at the beginning of n+2. The second version looks at the current time (down to the millisecond), and tries to schedule the next call of display() just after the start of each second, which results in a much smoother display.

Upvotes: 0

poke
poke

Reputation: 387667

There are some smaller mistakes:

  1. minutes = Math.floor(temp / 60)";
    There is a trailing quotation mark that shouldn't be there.
  2. var temp=timeleft;
    That should be time_left.
  3. Also you should use setTimeout with a reference to the function instead of requiring it to parse the text as code: setTimeout(display,1000);

edit: I wrote a small, clearer countdown script.

function startCountdown ( timeLeft )
{
    var interval = setInterval( countdown, 1000 );
    update();

    function countdown ()
    {
        if ( --timeLeft > 0 )
            update();
        else
        {
            clearInterval( interval );
            update();
            completed();
        }
    }

    function update ()
    {
        hours   = Math.floor( timeLeft / 3600 );
        minutes = Math.floor( ( timeLeft % 3600 ) / 60 );
        seconds = timeLeft % 60;

        alert( '' + hours + ':' + minutes + ':' + seconds );
    }

    function completed ()
    {
        alert( 'completed' );
    }
}
startCountdown( 1234 );

Just alter the update and completed function to your needs and start the countdown with your desired value.

And btw. if you want to change some innerHTML of an element in your code, you should save the reference somewhere, instead of getting it via DOM every call.

Upvotes: 4

Gabriele Petrioli
Gabriele Petrioli

Reputation: 195992

All the typos that have been mentioned above and one logic flaw ..

If the code is in the head section of the html then the first time you call the display() the timeframe element has not been created yet..

instead of display(); use window.onload=display; to run it for the first time, after the dom has loaded..

here is a fixed working version of your code.. http://jsbin.com/orege/edit

Upvotes: 1

T.J. Crowder
T.J. Crowder

Reputation: 1074285

One thing that jumps out at me is that your calculation at the top (starting with var temp...) should be inside the display function (which probably wants a new name). Where it is, it's only done once. (I'm also not convinced that the actual calcuation is correct. In fact, the code overall is not really ideal, you might not want to get more code from wherever you got this. But I'm not going to rewrite it for this answer.)

See also n00b32's answer, there seems to be a syntax error near the top.

Also (OT): You can change your setTimeout to: setTimeout(display, 1000). No need for the quotes and parens.

Upvotes: 0

n00b
n00b

Reputation: 5722

      minutes = Math.floor(temp / 60)";

?

Upvotes: 3

Related Questions