-
Notifications
You must be signed in to change notification settings - Fork 36
Refactor - split into Reader and Writer classes #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
5a25d00
fb574c8
9018818
68ab0a6
ca998d7
19e0c81
1f1589c
92f45cb
fe2a040
3c5325a
2161707
4179ac8
ca0e242
17fc0cc
fda01c0
92e590f
9a107c5
9d5ca09
9387be9
7325572
69c84b8
ed78d27
c5b9822
d09b013
1661411
e1d5fe7
035e737
537dbb0
7bd505d
f68c799
5b9ccb0
eb55800
572371a
a101d2f
44a4914
bb673a8
0916f78
3bc11a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| <?php | ||
|
|
||
| namespace Keboola\Csv; | ||
|
|
||
| use SplFileInfo; | ||
|
|
||
| class AbstractCsvFile extends SplFileInfo | ||
| { | ||
| const DEFAULT_DELIMITER = ','; | ||
| const DEFAULT_ENCLOSURE = '"'; | ||
|
|
||
| /** | ||
| * @var string | ||
| */ | ||
| protected $delimiter; | ||
|
|
||
| /** | ||
| * @var string | ||
| */ | ||
| protected $enclosure; | ||
|
|
||
| /** | ||
| * @var string | ||
| */ | ||
| protected $lineBreak; | ||
|
|
||
| /** | ||
| * @param string $delimiter | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| protected function setDelimiter($delimiter) | ||
| { | ||
| $this->validateDelimiter($delimiter); | ||
| $this->delimiter = $delimiter; | ||
| } | ||
|
|
||
| /** | ||
| * @param string $delimiter | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| protected function validateDelimiter($delimiter) | ||
| { | ||
| if (strlen($delimiter) > 1) { | ||
| throw new InvalidArgumentException( | ||
| "Delimiter must be a single character. \"$delimiter\" received", | ||
| Exception::INVALID_PARAM, | ||
| null, | ||
| Exception::INVALID_PARAM_STR | ||
| ); | ||
| } | ||
|
|
||
| if (strlen($delimiter) == 0) { | ||
| throw new InvalidArgumentException( | ||
| "Delimiter cannot be empty.", | ||
| Exception::INVALID_PARAM, | ||
| null, | ||
| Exception::INVALID_PARAM_STR | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @param string $enclosure | ||
| * @return $this | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| protected function setEnclosure($enclosure) | ||
| { | ||
| $this->validateEnclosure($enclosure); | ||
| $this->enclosure = $enclosure; | ||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * @param string $enclosure | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| protected function validateEnclosure($enclosure) | ||
| { | ||
| if (strlen($enclosure) > 1) { | ||
| throw new InvalidArgumentException( | ||
| "Enclosure must be a single character. \"$enclosure\" received", | ||
| Exception::INVALID_PARAM, | ||
| null, | ||
| Exception::INVALID_PARAM_STR | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @return string | ||
| */ | ||
| public function getDelimiter() | ||
| { | ||
| return $this->delimiter; | ||
| } | ||
|
|
||
| /** | ||
| * @return string | ||
| */ | ||
| public function getEnclosure() | ||
| { | ||
| return $this->enclosure; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,22 +2,10 @@ | |
|
|
||
| namespace Keboola\Csv; | ||
|
|
||
| class CsvFile extends \SplFileInfo implements \Iterator | ||
| class CsvReader extends AbstractCsvFile implements \Iterator | ||
| { | ||
| const DEFAULT_DELIMITER = ','; | ||
| const DEFAULT_ENCLOSURE = '"'; | ||
| const DEFAULT_ESCAPED_BY = ""; | ||
|
|
||
| /** | ||
| * @var string | ||
| */ | ||
| protected $delimiter; | ||
|
|
||
| /** | ||
| * @var string | ||
| */ | ||
| protected $enclosure; | ||
|
|
||
| /** | ||
| * @var string | ||
| */ | ||
|
|
@@ -44,9 +32,9 @@ class CsvFile extends \SplFileInfo implements \Iterator | |
| protected $currentRow; | ||
|
|
||
| /** | ||
| * @var string | ||
| * @var array | ||
| */ | ||
| protected $lineBreak; | ||
| protected $header; | ||
|
|
||
| /** | ||
| * CsvFile constructor. | ||
|
|
@@ -70,6 +58,7 @@ public function __construct( | |
| $this->setDelimiter($delimiter); | ||
| $this->setEnclosure($enclosure); | ||
| $this->setSkipLines($skipLines); | ||
| $this->openCsvFile(); | ||
| } | ||
|
|
||
| public function __destruct() | ||
|
|
@@ -79,7 +68,7 @@ public function __destruct() | |
|
|
||
| /** | ||
| * @param integer $skipLines | ||
| * @return CsvFile | ||
| * @return CsvReader | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| protected function setSkipLines($skipLines) | ||
|
|
@@ -105,71 +94,6 @@ protected function validateSkipLines($skipLines) | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * @param string $delimiter | ||
| * @return CsvFile | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| protected function setDelimiter($delimiter) | ||
| { | ||
| $this->validateDelimiter($delimiter); | ||
| $this->delimiter = $delimiter; | ||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * @param string $delimiter | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| protected function validateDelimiter($delimiter) | ||
| { | ||
| if (strlen($delimiter) > 1) { | ||
| throw new InvalidArgumentException( | ||
| "Delimiter must be a single character. \"$delimiter\" received", | ||
| Exception::INVALID_PARAM, | ||
| null, | ||
| Exception::INVALID_PARAM_STR | ||
| ); | ||
| } | ||
|
|
||
| if (strlen($delimiter) == 0) { | ||
| throw new InvalidArgumentException( | ||
| "Delimiter cannot be empty.", | ||
| Exception::INVALID_PARAM, | ||
| null, | ||
| Exception::INVALID_PARAM_STR | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @param string $enclosure | ||
| * @return $this | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| protected function setEnclosure($enclosure) | ||
| { | ||
| $this->validateEnclosure($enclosure); | ||
| $this->enclosure = $enclosure; | ||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * @param string $enclosure | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| protected function validateEnclosure($enclosure) | ||
| { | ||
| if (strlen($enclosure) > 1) { | ||
| throw new InvalidArgumentException( | ||
| "Enclosure must be a single character. \"$enclosure\" received", | ||
| Exception::INVALID_PARAM, | ||
| null, | ||
| Exception::INVALID_PARAM_STR | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @return string | ||
| * @throws Exception | ||
|
|
@@ -178,7 +102,6 @@ protected function detectLineBreak() | |
| { | ||
| rewind($this->getFilePointer()); | ||
| $sample = fread($this->getFilePointer(), 10000); | ||
| rewind($this->getFilePointer()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nespoléhal bych na to, že to nikdo nezavolá jen proto, že je to protected. Ta třída jde extendovat a budou z toho těžko odhalitelné chyby. Nebo co ji dát final?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tomu nerozumim, vadi ti, ze ma detectLineBreak side effect? no a co? nejmenuje se to getLineBreak (srovnej se soucasnou verzi, kde getLineBreak ma sideeffect), stejne tak ma openCsvfile a closeCsvFile maji side effect, pokud si tu tridu nekdo extenduje a zavola to nekde uprostred cteni/zapisu, tak je proste blbej nevidim smysl v tom snazit se tomu nejak zabranit
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok |
||
|
|
||
| $possibleLineBreaks = [ | ||
| "\r\n", // win | ||
|
|
@@ -218,33 +141,27 @@ protected function readLine() | |
| } | ||
|
|
||
| /** | ||
| * @param string $mode | ||
| * @return resource | ||
| * @throws Exception | ||
| */ | ||
| protected function getFilePointer($mode = 'r') | ||
| protected function getFilePointer() | ||
| { | ||
| if (!is_resource($this->filePointer)) { | ||
| $this->openCsvFile($mode); | ||
| } | ||
| return $this->filePointer; | ||
| } | ||
|
|
||
| /** | ||
| * @param string $mode | ||
| * @throws Exception | ||
| */ | ||
| protected function openCsvFile($mode) | ||
| protected function openCsvFile() | ||
| { | ||
| if ($mode == 'r' && !is_file($this->getPathname())) { | ||
| if (!is_file($this->getPathname())) { | ||
| throw new Exception( | ||
| "Cannot open file {$this->getPathname()}", | ||
| Exception::FILE_NOT_EXISTS, | ||
| null, | ||
| Exception::FILE_NOT_EXISTS_STR | ||
| ); | ||
| } | ||
| $this->filePointer = @fopen($this->getPathname(), $mode); | ||
| $this->filePointer = @fopen($this->getPathname(), "r"); | ||
| if (!$this->filePointer) { | ||
| throw new Exception( | ||
| "Cannot open file {$this->getPathname()} " . error_get_last()['message'], | ||
|
|
@@ -253,6 +170,10 @@ protected function openCsvFile($mode) | |
| Exception::FILE_NOT_EXISTS_STR | ||
| ); | ||
| } | ||
| $this->lineBreak = $this->detectLineBreak(); | ||
| rewind($this->getFilePointer()); | ||
|
||
| $this->header = $this->readLine(); | ||
| $this->rewind(); | ||
| } | ||
|
|
||
| protected function closeFile() | ||
|
|
@@ -262,22 +183,6 @@ protected function closeFile() | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * @return string | ||
| */ | ||
| public function getDelimiter() | ||
| { | ||
| return $this->delimiter; | ||
| } | ||
|
|
||
| /** | ||
| * @return string | ||
| */ | ||
| public function getEnclosure() | ||
| { | ||
| return $this->enclosure; | ||
| } | ||
|
|
||
| /** | ||
| * @return string | ||
| */ | ||
|
|
@@ -299,81 +204,18 @@ public function getColumnsCount() | |
| */ | ||
| public function getHeader() | ||
| { | ||
| $this->rewind(); | ||
| $current = $this->current(); | ||
| if (is_array($current)) { | ||
| return $current; | ||
| if ($this->header) { | ||
| return $this->header; | ||
| } | ||
|
|
||
| return []; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kdy to propadne sem? IMHO nikdy, ne?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. u prazdnyho souboru |
||
| } | ||
|
|
||
| /** | ||
| * @param array $row | ||
| * @throws Exception | ||
| */ | ||
| public function writeRow(array $row) | ||
| { | ||
| $str = $this->rowToStr($row); | ||
| $ret = fwrite($this->getFilePointer('w+'), $str); | ||
|
|
||
| /* According to http://php.net/fwrite the fwrite() function | ||
| should return false on error. However not writing the full | ||
| string (which may occur e.g. when disk is full) is not considered | ||
| as an error. Therefore both conditions are necessary. */ | ||
| if (($ret === false) || (($ret === 0) && (strlen($str) > 0))) { | ||
| throw new Exception( | ||
| "Cannot write to file {$this->getPathname()}", | ||
| Exception::WRITE_ERROR, | ||
| null, | ||
| Exception::WRITE_ERROR_STR | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @param array $row | ||
| * @return string | ||
| * @throws Exception | ||
| */ | ||
| public function rowToStr(array $row) | ||
| { | ||
| $return = []; | ||
| foreach ($row as $column) { | ||
| if (!( | ||
| is_scalar($column) | ||
| || is_null($column) | ||
| || ( | ||
| is_object($column) | ||
| && method_exists($column, '__toString') | ||
| ) | ||
| )) { | ||
| $type = gettype($column); | ||
| throw new Exception( | ||
| "Cannot write {$type} into a column", | ||
| Exception::WRITE_ERROR, | ||
| null, | ||
| Exception::WRITE_ERROR_STR, | ||
| ['column' => $column] | ||
| ); | ||
| } | ||
|
|
||
| $return[] = $this->getEnclosure() . | ||
| str_replace($this->getEnclosure(), str_repeat($this->getEnclosure(), 2), $column) . | ||
| $this->getEnclosure(); | ||
| } | ||
| return implode($this->getDelimiter(), $return) . "\n"; | ||
| } | ||
|
|
||
| /** | ||
| * @return string | ||
| * @throws Exception | ||
| */ | ||
| public function getLineBreak() | ||
| { | ||
| if (!$this->lineBreak) { | ||
| $this->lineBreak = $this->detectLineBreak(); | ||
| } | ||
| return $this->lineBreak; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Když už se to refaktoruje tak si myslím že by to němělo dědit ten
SplFileInfo, ale měl by být jako private property. To by mohlo umožnit např. založit ten reader/writer už z existujícího file resource. Na což jsem párkrát narazil a jsou na to i issues #14 a #5