1
\$\begingroup\$

Being new to raw JavaScript, intervals and times this was not the easiest script to come up with, and thus, although it seems to be working as I want it too the code looks very clunky.

The timer counts down to one of 4 times of day: 8am, 2pm, 8pm or 2am depending on which is the closest. Because they are at six hour intervals there can either be one or a maximum of 2 times that are currently in the future.

Can anyone suggest improvements to the code and how I might be able to make it more dynamic (more than 4 times of day for example)?

    <?php
                // define times to count down to and the time now
                $time1  = strtotime(gmdate("08:00:00")).'<br />';
                $time2  = strtotime(gmdate("14:00:00")).'<br />';
                $time3  = strtotime(gmdate("20:00:00")).'<br />';
                $time4  = strtotime(gmdate("02:00:00")).'<br />';
                $timeNow = strtotime('now').'<br />';

                // Get the difference between times
                echo $dif1 = $time1 - $timeNow.' 1<br>';
                echo $dif2 = $time2 - $timeNow.' 2<br>';
                echo $dif3 = $time3 - $timeNow.' 3<br>';
                echo $dif4 = $time4 - $timeNow.' 4<br>';
    ?>
    <script type="text/javascript">

        // 4 reboot times
        var time1 = "<?php echo $time1;?>"; // 8am
        var time2 = "<?php echo $time2;?>"; // 2pm
        var time3 = "<?php echo $time3;?>"; // 8pm
        var time4 = "<?php echo $time4;?>"; // 2am

        // time differences
        var dif1 = parseInt("<?php echo $dif1;?>");
        var dif2 = parseInt("<?php echo $dif2;?>");
        var dif3 = parseInt("<?php echo $dif3;?>");
        var dif4 = parseInt("<?php echo $dif4;?>");

        // count how many times are above zero
        var countDif = 0;

        // Check whether the time differences are above 0 (in the future)
        if(dif1>0){
            countDif = countDif+1;
        }
        if(dif2>0){
            countDif = countDif+1;
        }
        if(dif3>0){
            countDif = countDif+1;

        }
        if(dif4>0){
            countDif = countDif+1;
        }
        // if there is only one time in the future check which 1 it is
        if(countDif == 1){
            if(dif1>0){
                // set countdown to this time difference        
                var secs = dif1;
            }else if(dif2>0){
                // set countdown to this time difference    
                var secs = dif2;
            }else if(dif3>0){
                // set countdown to this time difference        
                var secs = dif3;
            }else if(dif4>0){
                // set countdown to this time difference        
                var secs = dif4;

            }
            // run the timer
            var counter1=setInterval(timer, 1000); //1000 will  run it every 1 second
        // if there are two counters in the future (max of two with 6 hour interval timers) check which two 
        }else if(countDif > 1){
            // check if dif 1 is greater than zero
            if(dif1>0){
                //check if dif2 2 is greater than zero
                if(dif2>0){
                    // check if dif1 is less than dif2
                    if(dif1<dif2){
                        // set countdown to this time difference        
                        var secs = dif1;
                    }else{
                        // set countdown to this time difference        
                        var secs = dif2;
                    }
                }else if(dif3>0){
                    // check if dif1 is less than dif3
                    if(dif1<dif3){
                        // set countdown to this time difference        
                        var secs = dif1;
                    }else{
                        // set countdown to this time difference        
                        var secs = dif3;
                    }

                }else if(dif4>0){
                    // check if dif1 is less than dif4
                    if(dif1<dif4){
                        // set countdown to this time difference        
                        var secs = dif1;
                    }else{
                        // set countdown to this time difference        
                        var secs = dif4;
                    }
                }
            }else if(dif2>0){
                //check if dif2 2 is greater than zero
                if(dif3>0){
                    // check if dif1 is less than dif2
                    if(dif2<dif3){
                        // set countdown to this time difference        
                        var secs = dif2;
                    }else{
                        // set countdown to this time difference        
                        var secs = dif3;
                    }
                }else if(dif4>0){
                    // check if dif1 is less than dif3
                    if(dif2<dif4){
                        // set countdown to this time difference        
                        var secs = dif2;
                    }else{
                        // set countdown to this time difference        
                        var secs = dif4;
                    }

                }
            }else if(dif3>0){
                //check if dif2 2 is greater than zero
                if(dif4>0){
                    // check if dif1 is less than dif2
                    if(dif3<dif4){
                        // set countdown to this time difference        
                        var secs = dif3;
                    }else{
                        // set countdown to this time difference        
                        var secs = dif4;
                    }
                }
            }   
            // run the timer
            var counter1=setInterval(timer, 1000); //1000 will  run it every 1 second
        }

        var a = 10;
        function timer()
        {

            if(secs == 0){
                //clearInterval(counter1);
                var announce=setInterval(announcement,1000);
            counter=setInterval(timer, 1000); //1000 will  run it every 1 second
            }else{

                var dh1 = Math.floor(secs/3600) % 24;
                var dm1 = Math.floor(secs/60) % 60;
                var ds1 = secs % 60; 

                secs=secs-1;
                document.getElementById("timer").innerHTML=dh1+"hrs "+dm1+"mins "+ds1+"secs "+"("+secs+")";
            }

        }

function announcement()
    {
        if(a==0){
            clearInterval(announce);
        }else{
            a=a-1;
            document.getElementById("timer").innerHTML="Server is rebooting...";
        }

    }


    </script>



    <span id="timer"></span>
\$\endgroup\$
4
\$\begingroup\$

Because they are at six hour intervals there can either be one or a maximum of 2 times that are currently in the future.

True, BUT there is still only 1 hour that is closes

This should do the same thing, and you could add more hours to the hours array.

What it will do is take the current hour (as of this writing: 16), compare it with the hours array and find whats next (in this case: 20), and then we start an interval which will check the time every 1 second.

var hours = [ 2, 8, 14, 20 ];
var currentHour = new Date().getHours();
var targetHour = 0;

for(var i = 0; i < hours.length; i++) {
    if(hours[i] > currentHour) { targetHour = hours[i]; break; }
}

var intervalID = setInterval(function() {
          console.log("running timer...");
          if(new Date().getHours() == targetHour) {
               alert("done");
               //
               /*  Here goes your logic for what should happen when your target hour hits */
               //   
               clearInterval(intervalID); //stop the interval once the hour has been hit  
      }
}, 1000);

edit

As Simon pointed out we need a way to either break out of the interval once the targeted hour has been hit or we need to make sure that the scope of if(new Date().getHours() == targetHour) is not executed every second once the targeted hour has been hit. In this case we invoke clearInterval on the interval ID returned by setInterval, effectively killing the interval.

Another alternative would be to use a var targetHourHit = false variable and only execute the scope of the if when our variable is false, and at the end set our variable to true.

\$\endgroup\$
  • 1
    \$\begingroup\$ I'd rather not see the same alert message once every second for one hour. But you're on the right track. \$\endgroup\$ – Simon Forsberg Dec 10 '13 at 15:33
  • \$\begingroup\$ @SimonAndréForsberg I'd rather not see an alert at all, or rarely - but it's a simple way of conveying a message to an user and it's a simple way of distinguishing when a unique or rare conditional has been met. \$\endgroup\$ – Max Dec 10 '13 at 15:35
  • \$\begingroup\$ This is great @Max thanks I assume I can use the same date class to easily display the current minutes and seconds, the same way you used gethours? \$\endgroup\$ – crmepham Dec 10 '13 at 15:40
  • \$\begingroup\$ @SimonAndréForsberg - Oh, I see your point now... Yeah, you're right. \$\endgroup\$ – Max Dec 10 '13 at 15:42
  • \$\begingroup\$ @crm - Yes you can! Check out w3schools.com/jsref/jsref_obj_date.asp for all functions in the Date object. \$\endgroup\$ – Max Dec 10 '13 at 15:43
4
\$\begingroup\$

Adding to the things Max wrote, to make sure that you don't make the same mistakes again I wanted to point out several things regarding your current code:

  • You're using both PHP and JavaScript. Why I am not exactly sure. You should be able to do everything with JavaScript only.
  • You're time variables gets set to strings, with an additional HTML <br /> element. Then you're setting them to JavaScript variables. The JavaScript time variables are unused
  • Your PHP dif variables are set to a string in PHP and then Parsed as an integer in JavaScript. Use only an integer variable and use it as part of an echo instead.
  • If the time is 01:00:00, won't all four times be in the future?
  • You are using four numbered variables for each thing, the next time you find yourself doing this think: Would using an array help here? Most often the answer is yes.
  • Whenever you write messy code like the if(countDif > 1) part, you are bound to miss a case, or forget to update one when you make a change, which is very likely to cause bugs. Since you wanted to get the lowest positive value, you could instead use this code:

    var lowest = 2000000000; // initialize to a very high value
    if ((dif1 > 0) && (dif1 < lowest)) lowest = dif1;
    if ((dif2 > 0) && (dif2 < lowest)) lowest = dif2;
    if ((dif3 > 0) && (dif3 < lowest)) lowest = dif3;
    if ((dif4 > 0) && (dif4 < lowest)) lowest = dif4;
    

    However, it is of course a lot better to use an array, as Max has used in his answer.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.