patternphpMinor
Testing uptime of my personal server
Viewed 0 times
servertestingpersonaluptime
Problem
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.
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:
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
Solution
Naming
You should choose longer variable names. For example, with names as
But all other names should be expressive as well.
For some names it's obviously more important than for others, but good naming is always an improvement.
Variable Scoping
It's closed in the
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.
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.
Context
StackExchange Code Review Q#64336, answer score: 2
Revisions (0)
No revisions yet.