JavaScript - Function returning twice

Question

I don't know much about JavaScript, here is the code I have:

<script language="JavaScript">
var x = 10

function startClock() {
    if (x !== ' ') {
        x = x - 1
        document.frm.clock.value = x
        setTimeout("startClock()", 1000)
    }
    if (x == 0) {
        x = ' ';
        document.frm.clock.value = x;
        success.location.href = "success.php";
    }
}
</script>
<body onLoad(startClock);>

affected iframe:

<input name="clock" size="3" readonly="readonly"

<iframe name="success" src="blank.htm"></iframe>

when the timer counts down, success.php is loaded twice. I know thise because 1.)It inserts data into my DB twice, 2.)I can actually see the loading symbol in the tab reloading a second.

When I change the function to something like:

 <script language="JavaScript">

var x = 10
var y = 1

function startClock() {
    if (x !== 'Fin') {
        x = x - y
        document.frm.clock.value = x
        setTimeout("startClock()", 1000)
    }
    if (x == 0) {
        x = 'Fin';
        document.frm.clock.value = x;
        success.location.href = "success.php";
    }
}

</script>

...the page is only loaded once.

Can anyone tell me what is happening here? I also tried using '0' in place of ' ' and got the same double execution...

Changing your second if to and else if might solve your problem. — Ivan
– Ivan, Commented Jan 11, 2012 at 23:20
"It inserts data into my DB twice." Then you may need to fix your server page. GET operations must be idempotent (sorry, not my word). For operations that change things, use POST or one of the other verbs. (The exception, in my opinion, being "visits" data.) — T.J. Crowder
– T.J. Crowder, Commented Jan 11, 2012 at 23:22
The first version of your question used the flag value " ". The current version uses "Fin". Which is it? Oh, wait, it's back to " " again. — T.J. Crowder
– T.J. Crowder, Commented Jan 11, 2012 at 23:23
this was itching me for some reason <body onload="startClock()"> — Ibu
– Ibu, Commented Jan 11, 2012 at 23:59

Murray McDonald · Accepted Answer · 2012-01-11 23:44:52Z

1

In Javascript there are TWO comparison operators:

"==" -- means equal to

"===" means "exactly equal to" -- which means that the value and the TYPE must be the same

I suspect (although I dind't bother to test the theory) that if you use "===" rather than "==" in your original code you will find it works as you intended. However, there are a number of things that need fixing -- 1) you are inconsistent with using ";", 2) the code should be structured to ensure that on any given iteration it can only "restart" the timer OR fire the sucess and NEVER both. Here is a cleaner version:

<script language="JavaScript">
   // 10 iterations at 1 second intervals
   var x = 10;

   function startClock() {
     document.frm.clock.value = --x;
     if (x <= 0) {
       document.frm.clock.value = x;
       success.location.href = "success.php";
     } else {
       setTimeout("startClock()", 1000);
     }
   } // startClock
</script>
<body onLoad(startClock);>

answered Jan 11, 2012 at 23:44

Murray McDonald

6314 silver badges5 bronze badges

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

5 Comments

Timothy Jones Over a year ago

+1 Nice answer. I think the second document.frm.clock.value = x; is superfluous though. Or maybe you meant = ' ';?

Murray McDonald Over a year ago

Yes, totally superluous -- just overlooked as I was moving stuff around -- in PHP 0 == '' is TRUE but 0 === '' is FALSE -- I know that the code quoted above is using ' ' (single blank rather than a null string) but the fact that when he changed his "sentinel" value from that to "Fin" it worked very much makes me think that's what's going on

user1142872 Over a year ago

Thanks for clearing up the '==' and '===' operators, something new learned. Thanks for the clean code, works great!

Murray McDonald Over a year ago

You're welcome -- as Tim pointed out the second assignment to document.frm.clock.value can be removed - take care

Timothy Jones Over a year ago

@user1142872: if Murray's answer solved the problem for you, you should mark it as accepted (there's a tick next to the answer score). Welcome again to stack overflow :)

Rich Wagenknecht · Accepted Answer · 2012-01-11 23:34:55Z

First a couple of things. There's a number of "sloppy" coding practices in your example (missing semicolons for instance). While the code may run, it could improve with some jslint help.

So look at the case when x = 1. You decrement x so now x = 0. You then call setTimeout which will wait 1 second and then call your method named startClock. However, setTimeout doesn't block your execution. So immediately after setTimeout is called with x = 0, the code below it is executed where you set x to ' ' (and load your page). Now one second after that code has run, your method is called again due to the timer firing. Since x is now ' ', the top block is skipped and you fall into x == 0 block a second time.

Timothy Jones · Accepted Answer · 2012-01-11 23:55:42Z

0

Change it to:

if (x == 0) {  // note this is now first
    x = ' ';
    document.frm.clock.value = x;
    success.location.href = "success.php";
} else if (x !== ' ') { // note the else/if
    x = x - 1;
    document.frm.clock.value = x;
    setTimeout("startClock()", 1000)
}

Otherwise, when x is 1, a timeout for startClock() will be set, AND the location will be loaded. Then, the timeout will fire, loading the page again (since x = ' ' and ' ' == 0 returns true).

It is probably better practice to say:

if (x === 0) {  // note the ===
    x = ' ';
    document.frm.clock.value = x;
    success.location.href = "success.php";
} else if (x !== ' ') {
    x = x - 1;
    document.frm.clock.value = x;
    setTimeout("startClock()", 1000)
}

Because you don't need the truth conversion that == does for you.

Your example with 'Fin' instead of ' ' worked, because on the startClock() call after the location had been loaded, x was 'Fin', and ('Fin' == 0) is false.

edited Jan 11, 2012 at 23:55

answered Jan 11, 2012 at 23:34

Timothy Jones

22.3k6 gold badges65 silver badges95 bronze badges

2 Comments

user1142872 Over a year ago

Just tried it, for some reason the else if statement is not being ran... The timer continues into the negative after it hits zero. It's as if x never equals 0???

Timothy Jones Over a year ago

Ah. The logic is wrong. I'll update the answer - but I'd recommend @Murray McDonald's version. His is cleaner. I gave him an upvote - and if you find his solution useful too, you should too :) (also welcome to Stack Overflow)

Collectives™ on Stack Overflow

JavaScript - Function returning twice

3 Answers 3

5 Comments

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

5 Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related