$local_dir = $_SERVER["DOCUMENT_ROOT"].'/all/app/Filescletec/glamorousnap/';
$local_server_dir = $_SERVER["DOCUMENT_ROOT"].'/all/app/Filescletec/copyfiles/';
$files = clean_scandir($local_dir);
if (isset($_POST['copy'])) {
for ($i=0; $i<count($files) ; $i++) {
if (!file_exists("$local_server_dir/$files[$i]")) {
if (copy("$local_dir/$files[$i]","$local_server_dir/$files[$i]")) {
echo "Succesfully copied"; echo " file #"; echo $files[$i]; echo "<br>";
}
}else {
echo "Record already exists!";
echo "<br>";
}
}
}
if (isset($_POST['purge'])) {
for ($i=0; $i<count($files) ; $i++) {
if (file_exists("$local_server_dir/$files[$i]")) {
unlink("$local_server_dir/$files[$i]");
echo "File Purged: ";echo $files[$i]; echo "<br>";
} else {
echo "No file exists to purge with the name: "; echo $files[$i]; echo "<br>";
}
}
}
I have to build a small app that copies file from one folder to another, and it works fine, but is it possible to write better code and minimize what I have written in isset part?
There is some repetition in both conditions, so it could be minimised, perhaps with a function with an action param, then possibly sub functions to perform the particular action.
This looks like a clear case for a foreach loop, rather than for.
I personally don’t like using double quotes unless I really have to (e.g. when using carriage returns) . To me, having double quotes everywhere is a bit messy and hard to read. All I would really see is double quote this, double quote that. That’s why I tend to use variables for their exact purpose so I can cut down on using unnecessary quotes. That’s just me though.
I was trying to create a working test and toggle FALSE with TRUE and it worked OK.
The temporary ‘…/kill-001’ directory had insufficient permissions to copy files and prepending @ to copy stopped the errors but required path permissions to be set to 0777. Once set the @ was not required to precede unlink.
You could use an anonymous function to encapsulate the processing you do on each file, then call that in a loop; that way you’d only need a single foreach loop. The pseudocode below is a summary:
if $_POST['copy'] is set:
$operation = function ($file) {
Validate file can be copied
and then copy it
};
else if $_POST['purge'] is set:
$operation = function ($file) {
Validate file can be deleted
and then delete it
};
foreach $file in $files:
call $operation with $file
However, you will probably find that this code ends up more and more separated as you continue to work on it and the two paths become more different. I’d advise against trying to shrink the code prematurely.
One thing I would suggest is that you remove the @ error suppression. Errors are great! They tell you that something’s wrong and where you need to start looking. PHP can be configured to output errors to the screen or to log them; all you need to do is make your development environment is set to display them on screen and your production environment is set to log them to a file for you to check later. This document has more information on doing that.
