Posted by curtis on May 30, 2008 at 11:28pm
| Project: | Stock API |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
I noticed that the symbol ^GSPC (S&P 500 INDEX,RTH) wasn't updating properly. After some investigation, I discovered it was because the CSV info wasn't being parsed correctly. Basically, the module code was simply splitting the returned data on every comma in the string. The problem, of course, is that some fields contained commas, which means data was being shifted unexpectedly.
This is an easy fix. All you have to do is save the CSV data to a temporary file, and then load it into an array using the built-in PHP function fgetcsv. See attached patch.
Thanks,
Curtis.
| Attachment | Size |
|---|---|
| stockapi.module.diff | 864 bytes |
Comments
#1
Well, that works, but I don't like the intermediate file thing. It is slow, and introduces concurrency issues.
Try this instead? http://php.net/manual/en/function.str-getcsv.php
Or something from comments there, or here http://www.php.net/manual/en/function.split.php
#2
I agree that it's a bit clumsy to save it to a temp file, but str_getcsv says:
(No version information available, might be only in CVS)So fgetcsv is really the only built-in PHP option.
Writing a separate CSV parsing function based on notes in the PHP manual comments is, of course, possible, but not something I will be doing.
Thanks,
Curtis.
#3
I fixed this bug without using a temporary file. It works on the assumption that commas that need to be escaped will always appear within a set of quotes. Insert the following code after (around line 107):
if ($result->code != 200) {watchdog('stockapi', t('StockAPI: drupal_http_request code: @code', array('@code' => $result->code)));
return FALSE;
}
Insert this code:
$data = $result->data;
$data = explode(',', $data);
$i = 0;
while ($i < (count($data) - 1)) {
$value = $data[$i];
if ((substr($value, 0, 1) == '"') && (substr($value, strlen($value) - 1, 1)) != '"') {
$data[$i] = $value . ',' . $data[$i+1];
$data = array_merge(array_slice($data, 0, $i+1), array_slice($data, $i+2));
$i = $i - 1;
}
else
$data[$i] = str_replace('"', '', $value);
$i++;
}
$data[$i] = str_replace('"', '', $data[$i]);
and before:
if (!$data) {watchdog('stockapi', t('StockAPI: no data for symbol(s): @symbol', array('@symbol' => $symbol)));
return FALSE;
}
It should at least fix the problems for the yahoo feed.
#4
Please submit a working patch for review. See http://drupal.org/patch for details on how to do so.
#5
Here's a patch with the code from andywalters.
KC
#6
Thank you for taking the time to create this.
I think that this code is a bit verbose and complicated. Can it be simplified as much of possible so it is maintainable and understandable going forward?
#7
Here's an updated simplified version of the patch.
KC
#8
Committed.
Thanks
#9
Automatically closed -- issue fixed for 2 weeks with no activity.