3
\$\begingroup\$

I'm relatively new to php and any advice is appreciated.

I have written a code to the most recently updated time for a folder in php. Can you please correct me if I am doing anything wrong.

And here the code I use:

function get_m_time_dir ($directory)
{
$last_modified_time = 0;
$handler = opendir ($directory);
while ($file = readdir ($handler))
{
    if ($file == '.' || $file == '..')
    {
        continue;
    }
    if (is_file ($directory . DIRECTORY_SEPARATOR . $file))
    {
        $filemtime = filemtime ($directory . DIRECTORY_SEPARATOR . $file);
        if ($filemtime > $last_modified_time)
        {
            $last_modified_time = $filemtime;
        }
    } else
    {
        $filemtime = get_m_time_dir ($directory . DIRECTORY_SEPARATOR . $file);
        if ($filemtime > $last_modified_time)
        {
            $last_modified_time = $filemtime;
        }
    }
}
return $last_modified_time;
}
\$\endgroup\$

3 Answers 3

1
\$\begingroup\$

Can you please correct me if I am doing anything wrong.

It depends on whether you care for the deleted files or not. If you do, you should also check the modification time for the directory itself, not only files in it.

The code itself could be also improved:

  • you can remove the duplicated condition
  • which in turn, can be substituted with max() function.
  • also you can utilize the glob() function that can save you that boring $file == '..' condition, as well as other unnecessary code.

So in the end the code could be

function get_m_time_dir ($directory)
{
    $last_modified_time = 0;
    $dirmtime = filemtime ($directory);
    foreach (glob("$directory/*") as $file)
    {
        if (is_file ($file))
        {
            $filemtime = filemtime ($file);
        } else {
            $filemtime = get_m_time_dir ($file);
        }
        $last_modified_time = max($filemtime, $dirmtime, $last_modified_time);
    }
    return $last_modified_time;
}
\$\endgroup\$
1
\$\begingroup\$

I would consider using SPL's RecursiveDirectoryIterator and RecursiveIteratorIterator for this. This would eliminate the need for the function itself to be called recursively (which is generally a good thing since PHP does not perform tail call recursion optimization).

function get_m_time_dir($path)
{
    $directory = new RecursiveDirectoryIterator(
        $path,
        FilesystemIterator::KEY_AS_PATHNAME | 
        FilesystemIterator::CURRENT_AS_FILEINFO | 
        FilesystemIterator::SKIP_DOTS
    );
    $iterator = new RecursiveIteratorIterator(
        $directory,
        RecursiveIteratorIterator::SELF_FIRST 
    );
    $resultFile = $iterator->current();
    foreach($iterator as $file) {
        if ($file->getMtime() > $resultFile->getMtime()) {
            $resultFile = $file;
        }
    }
    return $resultFile->getMtime();
}

Note that inside the array_reduce() closure, you are working with SplFileInfo objects which provide a wealth of information about the individual file they represent. Also note that this function would throw an UnexpectedValueException if the path provided to instantiate the RecursiveDirectoryIterator is not valid.

I would recommend gaining some familiarity with the SPL Library as, in addition to providing great object-oriented means for interacting with the file system, this library provides a collection of fundamental classes that any PHP developer should be aware of.

\$\endgroup\$
7
  • \$\begingroup\$ PHP Warning: array_reduce() expects parameter 1 to be array, object given \$\endgroup\$ Commented May 12, 2017 at 16:40
  • \$\begingroup\$ @YourCommonSense Would have guess that class implementing Traversable could go into array_reduce(), but apparently not :) Updated to reflect simple foreach loop. \$\endgroup\$
    – Mike Brant
    Commented May 16, 2017 at 15:28
  • \$\begingroup\$ Thank you. I wanted to test, whether this function is checking directories. Looks like not - it doesn't change the output if I delete a file. \$\endgroup\$ Commented May 16, 2017 at 15:55
  • \$\begingroup\$ @YourCommonSense It certainly should. Are you using a FAT file system (like Windows) where directory mtime does not change on directory content changes (i.e. renaming, adding, deleting files)? \$\endgroup\$
    – Mike Brant
    Commented May 16, 2017 at 16:22
  • \$\begingroup\$ @YourCommonSense Actually realized I didn't specify the RecursiveIteratorIterator::SELF_FIRST on RecursiveIteratorIterator so it was only doing leaf nodes previously. \$\endgroup\$
    – Mike Brant
    Commented May 16, 2017 at 16:40
-2
\$\begingroup\$

I think you can also do like

function get_m_time_dir ($directory)
{
$last_modified_time = 0;
$handler = opendir ($directory);
while ($file = readdir ($handler))
{
    if (is_file ($directory . DIRECTORY_SEPARATOR . $file) && !($file == '.' || $file == '..'))
    {
        $filemtime = filemtime ($directory . DIRECTORY_SEPARATOR . $file);
        if ($filemtime > $last_modified_time)
        {
            $last_modified_time = $filemtime;
        }
    } else
    {

       if !($file == '.' || $file == '..'){
             $filemtime = get_m_time_dir ($directory . DIRECTORY_SEPARATOR . $file);
          if ($filemtime > $last_modified_time)
          {
             $last_modified_time = $filemtime;
          }
       }
    }
}
return $last_modified_time;
}
\$\endgroup\$
1
  • \$\begingroup\$ You should offer some explanation in your answer, not just a code rewrite. What did you change? Why did you change it? \$\endgroup\$
    – Mike Brant
    Commented May 12, 2017 at 16:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.