7
\$\begingroup\$

I made some code to test my personal servers uptime. It logs in to my router page and sees if it returns status 200. A cron job runs it every 5 minutes. Please let me know if I can make the code more efficient.

<?php
//Date
date_default_timezone_set('America/Los_Angeles');
$date = date('m/d h:i:s a', time());

//Output
$myFile = dirname(__FILE__) . "/output.txt";
$fh = fopen($myFile, 'a+') or die("can't open file");

//Count #
$count_file = dirname(__FILE__) . "/OK.txt";
$fil = fopen($count_file, r);
$dat = fread($fil, filesize($count_file)); 
fclose($fil);
$fil = fopen($count_file, w);

$ch = curl_init();
curl_setopt($ch, CURLOPT_URL, "MYURL");
curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
curl_setopt($ch, CURLOPT_USERPWD, "USER:PASSWORD");
curl_setopt($ch, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false);
curl_setopt($ch, CURLOPT_FOLLOWLOCATION, true);
curl_setopt($ch, CURLOPT_UNRESTRICTED_AUTH, 1);
curl_exec($ch);
$retcode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
curl_close($ch);


if($retcode==200){
    //fwrite($fh, "$date\tOK\n");
    $OK = $dat + 1;
    fwrite($fil, $OK);
    echo "$date\tOK\n";
    fclose($fh);
} else {
    fwrite($fh, "$date\t\tDOWN\n");
    echo "$date\tDOWN\n";
    fclose($fh);
}

//Count OK/DOWN
$read = file_get_contents($myFile);
$DOWN = preg_match_all("/DOWN/", $read, $matches) - 1;
$UP = 100*bcdiv($OK,$OK+$DOWN,4);

//Remove first line 
$read = file($myFile);
array_shift($read);
$server_up = "Server Uptime: " . $UP . "%\nOK: " . $OK . ", DOWN: " . $DOWN . "\n";
$read[0] = $server_up;
file_put_contents($myFile, $read);

?>

One thing I wanted to do was to somehow assign $OK to a variable and retrieve it the next time the page is loaded but I do not think PHP can do this so I saved it to an external file (OK.txt) and I read the value every time and increment it every time my site returns 200.

Here is a sample output.txt file:

Server Uptime: 92.5%
OK: 37, DOWN: 3
09/30 06:48:28 pm DOWN
09/30 06:48:28 pm DOWN
09/30 06:48:28 pm DOWN
\$\endgroup\$
  • \$\begingroup\$ Why do you use fopen, fread in the first part of the code, then file_get_contents in the 2nd part? to me it would be simpler to use file_get_contents and file_put_contents, but I thought I would check incase you have a specific reason \$\endgroup\$ – bumperbox Oct 1 '14 at 2:16
  • \$\begingroup\$ I used fopen to use string operations on fwrite and i used file_get_contents so I could used preg_match_all to count occurences of "Down" \$\endgroup\$ – Bijan Oct 1 '14 at 2:20
  • \$\begingroup\$ short of breaking it into functions/methods (which is probably overkill), the code looks fine, its easy to read and understand, not sure it can be improved much \$\endgroup\$ – bumperbox Oct 1 '14 at 7:21
  • \$\begingroup\$ Put all of the curl options in an associative array, put it at the top of your script, and call curl_setopt_array($ch, $options);. One function call, all options are set + all options can be maintained more easily \$\endgroup\$ – Elias Van Ootegem Oct 1 '14 at 12:58
2
\$\begingroup\$

Naming

You should choose longer variable names. For example, with names as $fil and $fh it's hard to remember which one was which. You could name them $output and $count.

But all other names should be expressive as well. $dat might be $uptime_count, and $myFile -> $output_file_name, $fh -> $output_file, $count_file -> $count_file_name (it't just a string, not a file), $ch -> $curl, etc.

For some names it's obviously more important than for others, but good naming is always an improvement.

Variable Scoping

$fh is only used once, far away from where it was defined, inside the else statement. I would move the opening code here, so you don't have to think about $fh outside the else.

It's closed in the if as well, but that is unnecessary. Don't open up the file if you don't plan on writing to it.

Functions

The code isn't all that long, but I would still extract some functionality to its own function. Especially the counting of previous up and down times, and maybe the connecting to the server.

\$\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.