HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Recursive Directory Copy

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
recursivecopydirectory

Problem

I've been told that my code is horribly-written. It works exactly the way I want it to, but I have been told that it needs to be safer and more efficient.

Here's the code.

Here's the main PHP script:

```
0) {
echo "Sorry That Resume Name Has Been Taken Please Try Again :D";
} //mysql_num_rows($dbunames) > 0
else {
// function to recursively copy
// a directory and its subdirectories
function copyRecursive($source, $destination)
{
// check if source exists
if (!file_exists($source)) {
die("'$source' is not valid");
} //!file_exists($source)
if (!is_dir($destination)) {
mkdir($destination);
} //!is_dir($destination)
// open directory handle
$dh = opendir($source) or die("Cannot open directory '$source'");
// iterate over files in directory
while (($file = readdir($dh)) !== false) {
// filter out "." and ".."
if ($file != "." && $file != "..") {
if (is_dir("$source/$file")) {
// if this is a subdirectory

Solution

There is quite a lot that can be done

For a start use mysqli_ functions as mysql_ functions are deprecated

on database queries, it is good practice to check for errors (like this)

// $dbunames = mysql_query("SELECT * FROM users WHERE website='$website'");
$dbunames = mysql_query("SELECT * FROM users WHERE website='$website'") or die(mysql_error());


It is not necessary to put quotes around variables unless you are concatenating them. By the way $username is not set in your script.

// mysql_connect("$host", "$username", "$password") or die("cannot connect");
mysql_connect($host, $username, $password) or die("cannot connect");


This will cause an error notice if firstname is not set, check it exists like this

// $firstname         = $_POST['firstname'];
$firstname         = isset($_POST['firstname']) ? $_POST['firstname'] : '';


My personal preference is not to nest functions inside if/else statements as you have done with the copyRecursive($source, $destination) function. Put functions at the top level it will make your code easier to read.

For code readability I would also move your email generation code into a separate function

Code Snippets

// $dbunames = mysql_query("SELECT * FROM users WHERE website='$website'");
$dbunames = mysql_query("SELECT * FROM users WHERE website='$website'") or die(mysql_error());
// mysql_connect("$host", "$username", "$password") or die("cannot connect");
mysql_connect($host, $username, $password) or die("cannot connect");
// $firstname         = $_POST['firstname'];
$firstname         = isset($_POST['firstname']) ? $_POST['firstname'] : '';

Context

StackExchange Code Review Q#30472, answer score: 7

Revisions (0)

No revisions yet.