patternphpMinor
Copy a file, renaming it if the destination already exists.
Viewed 0 times
destinationfilethealreadyrenamingexistscopy
Problem
I would love some feedback on this little utility function:
/**
* Copy $src to $dest, renaming it if the desired destination already exists. The
* string '.versionX' is inserted before the file extension if a file must be
* renamed, with X incremented as needed.
*
* @return boolean true if copy was successful, false otherwise.
*/
public static function copy_with_rename($src, $dest)
{
if (file_exists($dest))
{
// See if it's the same.
$existingFileHash = sha1_file($dest);
$new_file_hash = sha1_file($src);
if ($existingFileHash != $new_file_hash)
{
// File of same name exists, but is different.
$last_dot_pos = strrpos($dest,'.');
$file_count = 2;
// So find a new name.
do
{
$base_name = substr($dest, 0, $last_dot_pos);
$ext = substr($dest, strlen($base_name)); // with dot
$new_name = $base_name.'.version'.$file_count.$ext;
$file_count++;
} while (file_exists($new_name) && sha1_file($src)!=sha1_file($new_name));
// And then try the copy again, with the new name.
return file::copy_with_rename($src, $new_name);
} else
{
return true;
}
} else
{
$dest_dir = dirname($dest);
// Create (to any depth) the destination directory, if required.
if (!file_exists($dest_dir))
{
mkdir($dest_dir, 0777, true);
}
// And finally do the actual copy.
return copy($src, $dest);
}
}Solution
Original Answer:
I'd put in some extra brackets in your while-loop for radability:
And you shouldn't set file-permissions to
Update for comment:
You should move out the sha_file($src) as this doesn't change but is recalculated each iteration. And that's the case why I don't like
I'd put in some extra brackets in your while-loop for radability:
while ( file_exists($new_name)
&& (sha1_file($src) != sha1_file($new_name))
);And you shouldn't set file-permissions to
777 by default (660 or even less). Everything else looks quite ok in my eyes (although I don't like the do/while construct, but I guess that's more a personal thing)Update for comment:
$srcSH1 = sha1_file($sec);
do
{
// ...
}
while ( file_exists($new_name)
&& (sha1_file($new_name) != $srcSH1)
);You should move out the sha_file($src) as this doesn't change but is recalculated each iteration. And that's the case why I don't like
do/while. Splits the $srcSH1 and the while-condition where it is used.Code Snippets
while ( file_exists($new_name)
&& (sha1_file($src) != sha1_file($new_name))
);$srcSH1 = sha1_file($sec);
do
{
// ...
}
while ( file_exists($new_name)
&& (sha1_file($new_name) != $srcSH1)
);Context
StackExchange Code Review Q#2647, answer score: 3
Revisions (0)
No revisions yet.