Php bad glob pattern

Question

I am getting images using this glob pattern, but the code looks really bad.

First I get all images which have the name screenshot + a number from 0-9 in an array, after that I get all screenshort + a number from 10-99 and so on.

I want to improve this code.

 $dirScreenshotsOne[] = glob($value.'/screenshot[0-9].png'); // 0-9
 $dirScreenshotsTwo[] = glob($value.'/screenshot[0-9][0-9].png'); // 10-99
 $dirScreenshotsThree[] = glob($value.'/screenshot[0-9][0-9][0-9].png'); // 100-999
 $dirScreenshotsAll = array_merge(
 $dirScreenshotsOne,
 $dirScreenshotsTwo,
 $dirScreenshotsThree);
 foreach($dirScreenshotsAll as $screenshotKey => $screenshotValue)
 ...

Answer 1

Consider working with SPL's fileinfo and iterator classes (i.e. SplFileInfo, DirectoryIterator and FilterIterator, CallbackFilterIterator, etc) to build something a little more resilient.

Example:

// your filter logic
$imageFilter = function (SplFileInfo $splFileInfo) {
 return preg_match(
 '/^screenshot[0-9]{1,3}\.png$/',
 $splFileInfo->getFilename()
 ) ? true : false;
}
// get files in directory based on filter function
try {
 $directoryIterator = new DirectoryIterator($directoryPath);
 $filteredImageIterator = new CallbackFilterIterator(
 $directoryIterator,
 $imageFilter
 );
} catch (Exception $e) {
 // do something here, or don't wrap in try-catch at all if you want to bubble up exception.
}
foreach($filteredImageIterator as $image) { /* your code */ }

Note that you can build upon the filter shown here by adding things like verifying file is readable/writable, get file modification timestamps, or other such functionality exposed by SplFileInfo, which could include ability to easily create SplFileObject for working with file contents.

Answer 2

Although cached, multiple filesystem requests are better to be avoided.

Assuming there should be not so much false positives, I would load all files that roughly match the pattern and then filter out the wrong ones in PHP

$dirScreenshots = glob($value.'/screenshot[0-9]*.png');
foreach ($dirScreenshots as $screenshotValue)
{
 if (!preg_match('~/screenshot[0-9]{1,3}\.png$~')) continue;
 ...
}

Answer 3

<?php
$globs = [
 glob($value.'/screenshot[0-9].png'),
 glob($value.'/screenshot[0-9][0-9].png'),
 glob($value.'/screenshot[0-9][0-9][0-9].png')
];
$all_dir_screenshots = array_merge(...$globs);

This solution above uses array_merge(...$array) which can be used to flatten an array, My advice would be to not use this technique of flattening, unless you are sure the top level of the target array contains just arrays.

Its advantage is that it's succinct in length, its disadvantage is that it does not telegraph intent of code very well, I would suggest naming the left variable name well to better convey the flattening operation in the middle

• Vogel612: You have presented an alternative solution, but haven't reviewed the code. Please explain your reasoning (how your solution works and how it improves upon the original) so that the author can learn from your thought process.

Answer 4

<?php
$screenshot_pattern = '{' . 
 $screenshot_dir . '/screenshot[0-9].png,' . 
 $screenshot_dir . '/screenshot[0-9][0-9].png,' . 
 $screenshot_dir . '/screenshot[0-9][0-9][0-9].png' .
'}';
$all_dir_screenshots = glob($screenshot_pattern, GLOB_BRACE);

I believe it's possible to chain glob regex together using the syntax above