csv
csv copied to clipboard
Generic exceptions should be replaced with more specific exceptions
Really tidy little CSV parser, my only issue is if I'm wanting to catch errors and return my own messaging to users, I'd currently have to catch the RuntimeException
and then check the message to determine what kind of failure it was.
I think it might be nicer to throw things like InvalidRecordException
or StreamNotSeekableException
which all extend a common ParserException
which can then extend RuntimeException
for backwards compatibility.
Happy to do a PR if you think it's a good idea, and thanks for publishing this!
And another thought about being able to access the actual validation errors if a record fails validation; at the moment you just check passes()
on the Laravel validator here:
https://github.com/offdev/csv/blob/master/src/Parser.php#L281
Maybe the Validator
class could mirror the behavior of the validate()
method of the underlying Laravel validator instead, which throws a ValidationException
with access to the validator so you can pull out the validation messages?
It could look something like this:
try {
$this->validator->validate($record);
$this->parseSuccess($record);
} catch (ValidationException $exception) {
$record->setIsValid(false);
$this->parseFailed($record, $exception);
}
and parseFailed
can then re-throw the exception if throws
is enabled. Anything upstream catching those ValidationException
exceptions can just do $exception->getValidator()
or similar to get the Validator and access the error messages.
Sorry for the delayed answer here, but I completely didn't see the open issues...
B2T: I think you made some valid points there. If you want, you can do a PR. If you don't want to, just let me know, so I can do it myself.