PHP script that combines data from two CSV files to produce two new files

Question

I've spent some time looking at various posts on Stack Overflow regarding PHP CSV file manipulation and have devised a script that combines data from two CSV files to create two new files.

The aim of the task was to combine voucher numbers from one file to the customer data in another, along with the current date. I achieved this by extracting all the necessary data and writing it to a new file.

As some additional security/record keeping I've also combined the voucher data with the customer ID and current date to another file, for reference if needed.

I'm relatively new to PHP so your feedback would be greatly appreciated so that I might improve my code/logic:

// Open files for reading
$customers = fopen("customerTest1.csv", "r");
$vouchers = fopen("vouchersExtra.csv", "r+");

// Create new files to write data
$customerOutput = fopen("customerUpdated.csv", "w");
$voucherOutput = fopen("vouchersUpdated.csv", "w");

// Get columns from customer data and voucher data
$customerColumns = fgetcsv($customers, 1000);
$voucherColumns = fgetcsv($vouchers, 1000);

// Add extra columns for the output files
array_push($customerColumns, "voucherNumber", "dateApplied");
array_push($voucherColumns, "id", "dateUsed");

// Write the column headers to both customer and voucher output files
if (!$voucherOutput && !$customerOutput) {
    die("$voucherOutput and $customerOutput BROKE");
} else {
    fputcsv($customerOutput, $customerColumns);
    fputcsv($voucherOutput, $voucherColumns);
}

// Open the input
if (!$customers) {
    die("$customers BROKE");
} else {

    while (!feof($customers)) {

        // Extract current customer row of data
        $currentCustomer = fgetcsv($customers, 1000);

        // Only run the code if a current customer row exists
        if ($currentCustomer != false) {

            // Extract current voucher row of data
            $currentVoucher = fgetcsv($vouchers, 1000);

            // Get current customer's ID
            $customerId = $currentCustomer[0];

            // Merge customer and voucher data and push a date string to the end of the array
            $newCustomerRow = array_merge($currentCustomer, $currentVoucher);
            array_push($newCustomerRow, date("l jS \of F Y h:i:s A"));

            // Merge voucher data, customer ID and push a date string to the end of the array
            $newVoucherRow = [];
            array_push($newVoucherRow, $currentVoucher[0], $customerId, date("l jS \of F Y h:i:s A"));

            // Write new customer data to output file
            fputcsv($customerOutput, $newCustomerRow);

            // Write new voucher data to output file
            fputcsv($voucherOutput, $newVoucherRow);

        }

    }

}

Answer 1

There are a few things you should take into account.

1. You never close your files. At the end, use fclose;
2. Use try {...} catch(){} finally {} and close the file(s) in the finally
3. Use functions to enhance readability
4. Do not use die or exit, this kills the script and is bad practise. See this blog post, very valuable
5. It might be wise to put "magic strings" in a constants file and including this with require - for example your datetime formats, csv columns and so forth. This makes sure that you do not have to edit the script when any of these would change but rather only change the constants.
6. Comments rot very quickly, any change would leave you with "you have to update the comments too". Don't use comments, in stead use functions that have self explanatory names. i.e. pushCustomerDateToCsvRow(param1, param2,... )

Answer 2

Note: I haven't tested this code. It might have some bugs or typos.

Using an object oriented API will make your code cleaner:

class CSVFileObject extends \SPLFileObject implements IteratorAggregate {

    /**
     * @var string
     */
    private $delimiter;

    /**
     * @var string
     */
    private $enclosure;

    /**
     * @param string $file the file name
     * @param string $delimiter CSV column Delimiter
     * @param string $enclosure special chars enclosure
     */

    public function __construct( $file , $openMode = 'r' , $delimiter = ',' , $enclosure = '"' ){
        parent::__construct( $file , $openMode );

        $this->delimiter = $delimiter;
        $this->enclosure = $enclosure;
    }

    /**
     * @return CSVIterator
     */

    public function getIterator(){
        return new CSVIterator( $this );
    }

    /**
     * @return array
     */

    public function getCSVLine(){
        return $this->fgetcsv( $this->getLine() , $this->delimiter , $this->enclosure );
    }

    /**
     * @return integer
     */
    public function writeCSVLine(array $data){
        return $this->fputcsv( $data , $this->delimiter , $this->enclosure );
    }
}

class CSVIterator implements Iterator {

    /**
     * File handler
     * @var CSVFileObject
     */
    private $csvFile;

    /**
     * Current line number
     * @var integer
     */
    private $key = -1;

    /**
     * Current line data
     * @var array
     */
    private $current = array();

    /**
     * Constructs a new CSV Iterator
     * @param CSV $csv O manipulador do arquivo CSV
     */

    public function __construct( CSVFileObject $csvFile ){
        $this->csvFile = $csvFile;
        $this->rewind();
    }

    /**
     * @return array
     */
    public function current(){
        return $this->current;
    }

    /**
     * Recupera a linha atual
     * @return integer
     */
    public function key(){
        return $this->key;
    }

    /**
     * Avança para o próximo elemento
     */
    public function next(){
        $this->current = $this->csvFile->getCSVLine();

        ++$this->key;
    }

    /**
     * @return void
     */

    public function rewind(){
        $this->key = -1;
        $this->current = array();
        $this->csvFile->seek( 0 );
        $this->next();
    }

    /**
     * @return boolean
     */
    public function valid(){
        return !$this->csvFile->eof();
    }
}

Then you can do:

$originalCustomerFile = new CSVFileObject("/path/to/originalCustomer.csv");
$originalVoucherFile = new CSVFileObject("/path/to/originalVoucher.csv");

$customerIt = $originalCustomerFile->getIterator();
$voucherIt = $originalVoucherFile->getIterator();

$customerIt->next();
$voucherIt->next();

if (!$customerIt->valid()) {
    throw new Exeption("Customer file is empty");
}

if (!$voucherIt->valid()) {
    throw new Exeption("Voucher file is empty");
}

$customerHeaders = $customerIt->current();
$voucherHeaders = $voucherIt->current();

// ...

$customerOutputFile = new CSVFileObject("/path/to/customer-output.csv", "w+");
$voucherOutputFile = new CSVFileObject("/path/to/voucher-output.csv", "w+");

$customerOutputFile->writeCSVLine($customerHeaders + ["voucherNumber", "dateApplied"]);
$voucherOutputFile->writeCSVLine($customerHeaders + ["id", "dateUsed"]);

$customerIt->next();
$voucherIt->next();
while ($customerIt->valid() && $voucherIt->valid()) {
    $currentCustomer = $customerIt->current();
    $currentVoucher = $voucherIt->current();

    $customerId = $currentCustomer[0];

    $newCustomerRow = $currentCustomer + $currentVoucher + [date("l jS \of F Y h:i:s A")];

    $newVoucherRow = [$currentVoucher[0], $customerId, date("l jS \of F Y h:i:s A")];

    $customerOutputFile->writeCSVLine($newCustomerRow);
    $vohcerOutputFile->writeCSVLine($newVoucherRow);

    $customerIt->next();
    $voucherIt->next();
}

Since now we're using the OO API, we don't have to worry about opening and closing the file, since this is handled by constructors and destructors.