3
\$\begingroup\$

I have the following code that reads a .csv file that than outputs the result in the form of a HTML table:

$fhdrop = fopen("ac.csv", "r");

while (!feof($fhdrop) ) {

$at[] = fgetcsv($fhdrop, 1024);

}

$fhdrop2 = fopen("rc.csv", "r");

while (!feof($fhdrop2) ) {

$at2[] = fgetcsv($fhdrop2, 1024);
}
?>

<table border=1>
<tr>
<td>a</td>
<td>b</td>
<td>c</td>
<td>d</td>
<tr>
<td><?php echo $at[0][0] ?></td>
<td><?php echo $at[0][1] ?></td>
<td><?php echo number_format($at[0][1]*$at2[0][1],2) ?></td>
<td><?php echo number_format($at[0][1]*$at2[1][1],2) ?></td>
</tr>
<tr>
<td><?php echo $at[1][0] ?></td>
<td><?php echo $at[1][1] ?></td>
<td><?php echo number_format($at[1][1]*$at2[0][1],2) ?></td>
<td><?php echo number_format($at[1][1]*$at2[1][1],2) ?></td>
</tr>
<tr>
<td><?php echo $at[2][0] ?></td>
<td><?php echo $at[2][1] ?></td>
<td><?php echo number_format($at[2][1]*$at2[0][1],2) ?></td>
<td><?php echo number_format($at[2][1]*$at2[1][1],2) ?></td>
</tr>
<tr>
<td><?php echo $at[3][0] ?></td>
<td><?php echo $at[3][1] ?></td>
<td><?php echo number_format($at[3][1]*$at2[0][1],2) ?></td>
<td><?php echo number_format($at[3][1]*$at2[1][1],2) ?></td>
</tr>
</table>

The contents of ac.csv are:

a, 5,
b, 10,
c, 24,
d, 21

The contents of rc.csv are:

not, 1.87,
notatall, 1.78

As you can guess from the original code, I need to do multiplication in the second column. In the second row and third column, for instance, I need 5*1.87 and 5*1.78. So basically a * not and a * notatall.

My code is not very efficient. How can I make it more efficient?

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Looping on feof is wrong. feof doesn't return return true until after you've hit the end of file. If the read pointer is sitting past the last character and you try to read, feof will return false, but the read will fail. At that point feof will return true.

Example:

1, a, b, c
2, a, b, c
3, a, b, c

Then reading it:

$rows = array();
while (!feof($fh)) {
    $rows[] = fgetcsv($fh);
}
print_r($rows);

A much more natural (and correct) loop structure is as follows:

while (($row = fgetcsv($fh)) !== false) {
    $rows[] = $row;
}

(You can of course use fgetcsv($fh, 1024) instead of fgetcsv($fh). Just an example.)


As far performance goes, you're doing what you're doing as fast as it can be done. In other words, to get a speed increase, the approach has to change, not the implementation.

There are three main approaches that you can take (that can all be combined as desired): use caching, put the HTML output straight into the first loop so you only do one loop instead of two, and try using a fixed field-size format rather than CSV so parsing overhead can be eliminated.

Putting the HTML output straight into the read loop is very gross from a separation of concerns perspective, and the gains would be minimal compared to the code quality hit.

Likewise, changing to a fixed size format would be a fairly minimal performance gain for quite a bit of headache.

Caching is what you need to do. A primitive approach is to just create an on-disk version of the array and read that rather than the CSV. A slightly more complex but much faster approach is to cache the CSV in memory using apc or even something like memcached (memcached is way overkill, as is arguably apc). Yet another approach is cache the output HTML rather than the array. This allows you to serve a static page rather than regenerate it each request. You can't get any faster than that.

Since the idea remains largely the same, I'll show an example of the simplest one to implement: caching the array.

$csvFilePath = "...";
$cacheFilePath = "...";

if (!file_exists($cacheFilePath) || filemtime($csvFilePath) > $cacheFilePath) {
    $fh = fopen($cacheFilePath, 'r');
    $rows = array();
    while (($row = fgetcsv($fh)) !== false) {
        $rows[] = $row;
    }
    file_put_contents($cacheFilePath, serialize($rows));
} else {
    $rows = unserialize(file_get_contents($cacheFilePath));
}

Though this one is very simple, it's also the least effective. I would cache the entire page output. I would probably use something like Zend_Cache or Symfony's caching component to avoid lots of nasty manual output buffer management. Alternatively, you could write your own simple page cache.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.