userresu
userresu

Reputation: 107

Two counters on one page

I'm struggling putting three counters on a single page using JavaScript. When I run this page, counters appear but they're freezed, don't refresh. Here is my code:

<!DOCTYPE html>
<head>
	<meta charset="utf-8"/>
	<title>Counters</title>

	<script type="text/javascript">
		function zero(element) {
		    if (element < 10) return element = "0" + element;
		    return element;
		}
		 
		function counter(day, month, id) {
		    var year =  (new Date).getFullYear();
		    var today = new Date(); 
		    var vDate = new Date(year,month-1,day,9,00); 
		    var ms_day = 24 * 60 * 60 * 1000; 
		 
		    var diff = (vDate.getTime() - today.getTime()); 
		    var e_daysLeft = diff / ms_day; 
		    var daysLeft = Math.floor(e_daysLeft);
		 
		    var e_hoursLeft = (e_daysLeft - daysLeft)*24;
		    var hoursLeft = Math.floor(e_hoursLeft);
		 
		    var e_minutesLeft = ((e_hoursLeft - hoursLeft)*60);
		    var minutesLeft = Math.floor(e_minutesLeft);
		 
		    var e_secondsLeft = Math.floor((e_minutesLeft - minutesLeft)*60);
		    var secondsLeft = Math.floor(e_secondsLeft);
		 
		    var text = 'Left:  '
		                    +daysLeft+' days, '
		                    +hoursLeft+ ' hours, '
		                    +minutesLeft+ ' minutes '
		                    +zero(secondsLeft)+' seconds!'
		 
		    var element = document.getElementById(id);
		 
		  
		    if (daysLeft+hoursLeft+minutesLeft+secondsLeft <= 0) {
		        element.innerHTML = 'Over' ;
		    } else {
		        element.innerHTML = text;
		        setTimeout("counter(day,month,id)",1000)
		    }
		}
		counter(14,2,'test7');
		counter(27,3,'test8');
		counter(30,6,'test9');

		</script>

<body onload="counter(14,2,'test7'); counter(27,3,'test8'); counter(30,6,'test9');">

	<div id="test7"></div>
	<div id="test8"></div>
	<div id="test9"></div>

</body>
</html>

Could you tell me why setTimeout("counter(day,month,id)",1000) is not working?

Upvotes: 0

Views: 166

Answers (3)

danShumway
danShumway

Reputation: 450

As other people have pointed out, you're passing in a string literal rather than a function.

That being said, a typical followup mistake that people often make is writing setTimeout(counter(day, month, id)). When you invoke a function (even as a parameter), that function is immediately invoked. From a runtime perspective, what that ends up looking like is this.

  • Call counter(day, month, id) and take the returned result of that call. In your program, this function eventually is going to end up returning null once it finishes looping through everything.
  • Call setTimeout(null, 1000), passing in the result of counter(day, month, id)

Calling setTimeout(counter(day, month, id)) is really just acting as a shorthand for:

var result = counter(day, month, id);
setTimeout(result, 1000);

This is why the other answers have suggested wrapping the function:

setTimeout(function () {
    counter(day, month, id);
}, 1000);

This allows you to still pass in all of your parameters to counter, while still passing a function with no parameters to setTimeout.

A cleaner solution (at least to me) is to use bind, which is kind of like a more convenient shorthand for the above method:

setTimeout(counter.bind(this, day, month, id), 1000);

It's outside of the scope of this answer, but the reason I like bind is that the parameters will be passed in by value rather than reference, so when using it you're less likely to run into a related common bug involving javascript closures inside of loops. It's just important for you to remember that bind is internally very similar conceptually to manually wrapping your methods.

In both instances, you're making a new function that invokes your target function for you with pre-defined parameters, and then passing that new function around rather than your original.

Upvotes: 1

Fran&#231;ois Richard
Fran&#231;ois Richard

Reputation: 7055

Try setTimeout( function() {counter(day,month,id)},1000) so it's a function which is given to setTimeout not a string.

Upvotes: 0

Stefan Koenen
Stefan Koenen

Reputation: 2337

setTimeout expects a function, not just a string, have a look here: http://www.w3schools.com/jsref/met_win_settimeout.asp

Here is a modified, working version: https://jsfiddle.net/3todtotp/

Counters

<script type="text/javascript">
    function zero(element) {
        if (element < 10) return element = "0" + element;
        return element;
    }

    function counter(day, month, id) {
        var year =  (new Date).getFullYear();
        var today = new Date(); 
        var vDate = new Date(year,month-1,day,9,00); 
        var ms_day = 24 * 60 * 60 * 1000; 

        var diff = (vDate.getTime() - today.getTime()); 
        var e_daysLeft = diff / ms_day; 
        var daysLeft = Math.floor(e_daysLeft);

        var e_hoursLeft = (e_daysLeft - daysLeft)*24;
        var hoursLeft = Math.floor(e_hoursLeft);

        var e_minutesLeft = ((e_hoursLeft - hoursLeft)*60);
        var minutesLeft = Math.floor(e_minutesLeft);

        var e_secondsLeft = Math.floor((e_minutesLeft - minutesLeft)*60);
        var secondsLeft = Math.floor(e_secondsLeft);

        var text = 'Left:  '
                        +daysLeft+' days, '
                        +hoursLeft+ ' hours, '
                        +minutesLeft+ ' minutes '
                        +zero(secondsLeft)+' seconds!'

        var element = document.getElementById(id);


        if (daysLeft+hoursLeft+minutesLeft+secondsLeft <= 0) {
            element.innerHTML = 'Over' ;
        } else {
            element.innerHTML = text;
            setTimeout(function(){counter(day,month,id)},1000)
        }
    }
    counter(14,2,'test7');
    counter(27,3,'test8');
    counter(30,6,'test9');

    </script>

<div id="test7"></div>
<div id="test8"></div>
<div id="test9"></div>

Upvotes: 1

Related Questions