Create a CSV from SQL Data and email as attachment using PHP

Question

I put together some functions that allows a PHP script to send the SQL data obtained from user inputs on a website via an email attachment as a CSV file. It works perfectly and I have no issues with the code. Alternatively with the right SQL query it would run every X amount of days with a cron job and only produce the data within a specific criteria such as every month.

The idea behind the code is for users that cannot extract their data from SQL and have limited experience running their businesses online. I would like some input from what I have put together from scripts and endless amounts of testing to see if the code is up to standard.

Firstly I create the fileName:

 function fileName() { $path = "csv/"; $filename = "Subscribers" . date('Ymd') . ".csv"; $filepath = $path . $filename; return $filepath; } 

After which I create the CSV file:

function createCSV() { include_once "conn.php"; global $conn; $filepath = fileName(); $output = fopen($filepath, "w"); fputcsv($output, array( 'Name', 'Email', 'Date Joined' )); $query = "SELECT name, email, dateTime FROM tableData"; $result = mysqli_query($conn, $query); while ($row = mysqli_fetch_assoc($result)) { fputcsv($output, $row); } return $output; fclose($output); } 

Followed by the code I cut and edited from a thread online for attaching the CSV using the built in mail function:

 function sendCSV() { // Recipient $to = '[email protected]'; // Sender $from = '[email protected]'; $fromName = 'Test'; // Email subject $subject = 'PHP Email with Attachment'; // Attachment file $file = fileName(); $files = glob('csv/*.csv'); unlink($files); createCSV(); // Email body content $htmlContent = ' <h3>PHP Email with Attachment</h3> <p>This email is sent from the PHP script with attachment.</p> '; // Header for sender info $headers = "From: $fromName" . " <" . $from . ">"; // Boundary $semi_rand = md5(time()); $mime_boundary = "==Multipart_Boundary_x{$semi_rand}x"; // Headers for attachment $headers .= "\nMIME-Version: 1.0\n" . "Content-Type: multipart/mixed;\n" . " boundary=\"{$mime_boundary}\""; // Multipart boundary $message = "--{$mime_boundary}\n" . "Content-Type: text/html; charset=\"UTF-8\"\n" . "Content-Transfer-Encoding: 7bit\n\n" . $htmlContent . "\n\n"; // Preparing attachment if (!empty($file) > 0) { if (is_file($file)) { $message .= "--{$mime_boundary}\n"; $fp = @fopen($file, "rb"); $data = @fread($fp, filesize($file)); @fclose($fp); $data = chunk_split(base64_encode($data)); $message .= "Content-Type: application/octet-stream; name=\"" . basename($file) . "\"\n" . "Content-Description: " . basename($file) . "\n" . "Content-Disposition: attachment;\n" . " filename=\"" . basename($file) . "\"; size=" . filesize($file) . ";\n" . "Content-Transfer-Encoding: base64\n\n" . $data . "\n\n"; } } $message .= "--{$mime_boundary}--"; $returnpath = "-f" . $from; // Send email $mail = @mail($to, $subject, $message, $headers, $returnpath); // Email sending status echo $mail ? "<h1>Email Sent Successfully!</h1>" : "<h1>Email sending failed.</h1>"; } 

I am quite new to PHP but managed to put something together that actually works. If you can point out my mistakes (which I do know there would be), or guide me to more simple techniques, I would really appreciate it and it will help me progress in my PHP learning path.

Answer 1

Global variable

It appears the connection string is used as a global variable:

global $conn; 

Global variables have more negative aspects than positives. The conn.php file could define a function that would return the connection represented by $conn And the function could be called where needed - e.g. in createCSV(). Then there would be no need to globally reference that variable and the function could be stubbed/mocked for unit testing.

Limit Data

It looks like this query is executed to get all data:

$query = "SELECT name, email, dateTime FROM tableData"; 

There is no indication of how muc. Then there would be no data exists but you should consider limiting data in some manner- either by adding WHERE conditions and/or a LIMIT clause. Otherwise selecting all rows has the potential to take a large amount of time.

String interpolation

The line to add the from header:

$headers = "From: $fromName" . " <" . $from . ">"; 

Uses string interpolation. That could be simplified to:

$headers = "From: $fromName <$from>"; 

conditionals

I see this:

if (!empty($file) > 0) { if (is_file($file)) { 

empty() returns a bool, so to compare a negated bool “greater than zero” seems like a convoluted comparison but would function. It would be simpler to just see if it is true so the type does not need to be converted:

if (!empty($file) === true) { 

Though with PHP this can be written simply as:

if (!empty($file)) { 

Then the next conditional can be combined using logical AND - i.e. &&

if (!empty($file) && is_file($file)) { 

This will keep indentation levels minimized.

After inspecting the return value of fileName() and how $file is not modified from the original value returned from that function, the first conditional expression seems pointless. It would make sense to have it if the value could change.