# Copy a file, renaming it if the destination already exists.

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);
}
}


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.

• Thanks for reviewing! I might just change the while() to do the two clauses separately, for easier reading: $already_exists = file_exists($new_name); $is_different = sha1_file($src) != sha1_file($new_name); } while ($already_exists && \$is_different); I know what you mean about do-while: I guess it's just not all that widely used (and familiar), so it can take a moment to see what's happening. But in this case, it's the cleanest way. And I'll change the permissions to 600 (don't know why the default for mkdir() is 0700; I was using this on Windows anyway, so didn't think about it). May 30 '11 at 3:29
• (erg. forgot that comments have crap formatting!) May 30 '11 at 3:30
• @Sam: It's fine to have that kind of statements in while-loops. Just make sure to format the loop correctly.
– Fge
May 30 '11 at 9:04
• Good point about moving the hash out of the loop — and then it can also be used in the if clause above. May 30 '11 at 23:50