patternphpMinor
Identifying most recent blogXX.php file
Viewed 0 times
filerecentphpidentifyingmostblogxx
Problem
I create blog posts as
Here's how I'm doing it, but I have the sense this could be much cleaner and more elegant:
People are always complaining that bad programming is too easy in PHP, and I suspect what I've posted is just that easy bad programming that makes people mad. Can someone tell me how to make this more PHP-like and more elegant? I've worked much more in Python, but I'd like to be less of a hack in PHP.
blog1.php...blog20.php...blog500.php etc and redirect users from blog.php (code below) to the most recent blog post as determined by the blog post with the highest number in the filename. Here's how I'm doing it, but I have the sense this could be much cleaner and more elegant:
$directory = "/var/www/html";
$scanned_directory = array_diff(scandir($directory), array('..', '.'));
$blogNumbers = Array();
foreach($scanned_directory as $file){
if(preg_match("/blog(?[0-9]+)/", $file, $matches)){
foreach($matches as $match){
if(is_numeric($match["digit"])) array_push($blogNumbers, $match["digit"]);
//had to put this numeric check in because otherwise
//script was pushing some 'b's onto array
}
}
}
$currentBlog = max($blogNumbers);
header('Location: http://www.sunnysideworks.nyc/Simply-Run/blog'.$currentBlog);
die();People are always complaining that bad programming is too easy in PHP, and I suspect what I've posted is just that easy bad programming that makes people mad. Can someone tell me how to make this more PHP-like and more elegant? I've worked much more in Python, but I'd like to be less of a hack in PHP.
Solution
One thing that makes me cringe is the indentation. It really is off.
I had to look 20 times to that stray brace to realize it isn't closing that
Your casing is inconsistent. You have
Same goes for
Pick one style and use it!
Same goes with quotes. You have single-quoted and double-quotes strings everywhere!
Just pick 1 style and use it! (My recommendation goes to single-quotes.)
You have the following line:
You should always use braces. Always! There have been serious bugs on iOS for this.
Why do you have a
You have the following line:
That sounds like a constant. Try something like this:
Or:
And use it like this:
Almost at the end, you have this:
After you fix your variables' casing, you can just do like this:
How I would write this:
Outside this review, that
I had to look 20 times to that stray brace to realize it isn't closing that
if(in_numeric()).Your casing is inconsistent. You have
array() and Array(). Pease, use array() only.Same goes for
$scanned_directory, $blogNumbers, $scanned_directory and $currentBlog.Pick one style and use it!
Same goes with quotes. You have single-quoted and double-quotes strings everywhere!
Just pick 1 style and use it! (My recommendation goes to single-quotes.)
You have the following line:
if(is_numeric($match["digit"])) array_push($blogNumbers, $match["digit"]);You should always use braces. Always! There have been serious bugs on iOS for this.
Why do you have a
die(); at the end? As soon as the script finishes, it will just exit. No need to have the die(); there.You have the following line:
$directory = "/var/www/html";That sounds like a constant. Try something like this:
define('DIRECTORY', '/var/www/html');Or:
const DIRECTORY = '/var/www/html';And use it like this:
$scanned_directory = array_diff(scandir(DIRECTORY), array('..', '.'));Almost at the end, you have this:
$currentBlog = max($blogNumbers);
header('Location: http://www.sunnysideworks.nyc/Simply-Run/blog'.$currentBlog);After you fix your variables' casing, you can just do like this:
header('Location: http://www.sunnysideworks.nyc/Simply-Run/blog' . max($blog_numbers));How I would write this:
define('DIRECTORY', '/var/www/html');
$scanned_directory = array_diff(scandir(DIRECTORY), array('..', '.'));
$blog_numbers = array();
foreach($scanned_directory as $file) {
if(preg_match('/blog(?[0-9]+)/', $file, $matches)) {
foreach($matches as $match) {
//had to put this numeric check in because otherwise
//script was pushing some 'b's onto array
if(is_numeric($match['digit'])) {
array_push($blog_numbers, $match['digit']);
}
}
}
}
header('Location: http://www.sunnysideworks.nyc/Simply-Run/blog' . max($blog_numbers));Outside this review, that
foreach doesn't seem necessary. But I don't know the actual names.Code Snippets
if(is_numeric($match["digit"])) array_push($blogNumbers, $match["digit"]);$directory = "/var/www/html";define('DIRECTORY', '/var/www/html');const DIRECTORY = '/var/www/html';$scanned_directory = array_diff(scandir(DIRECTORY), array('..', '.'));Context
StackExchange Code Review Q#97146, answer score: 7
Revisions (0)
No revisions yet.