HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Identifying most recent blogXX.php file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filerecentphpidentifyingmostblogxx

Problem

I create blog posts as 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 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.