-
-
Notifications
You must be signed in to change notification settings - Fork 342
Implementing 9.0 Public API #210
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- The Statement and the RecordSet class are introduced to improve Reader usage and behaviour - The Stream filtering is now supported for every object except when created from SplFileObject. - Stream Filtering is simplified and improve for Writer class
- AbstractCsv::setFlushThreshold is added to improve Writing speed - StreamIterator::fflush is added to improve Writing speed - StreamIterator::__clone to forbid the object cloning - StreamTrait improve
- Statement is made immutable - Removed newReader and newWriter methods - newLine and flushtreshold are only available on the Writer class - fetchDelimitersOccurence is only available on the Reader class
- remove Reader::fetch* methods - remove Reader::to* methods - RecordSet::toXML methods takes into account the header if presents
3391dc5 to
bbfadce
Compare
- Replacing InvalidRowException with a more generic exception InsertinException - insertOne and insertAll returns int
You can no longer remove are clear the stream filters It is done automatically when the class is destruct
- Remove StreamTrait - Bugfix header from AbstractCsv::output to be HTTP/2 compliant
386e466 to
20b5e8d
Compare
- Adding RecordFormatterInterface - Adding RecordValidatorInterface - Adding CallableValidatorAdapter to reduce BC breakage - Adding CallableFormatterAdapter to reduce BC breakage - Removing the Config subnamespace
20b5e8d to
9987799
Compare
3249cfd to
44e2097
Compare
- Remove RecordFormatterInterface - Remove RecordValidatorInterface restore callable usage instead
Adding a fourth parameter to indicate the header attribute name added to each record field when the header has been given to the CSV Document
Removed the header method and replaced it with the better columns method
Adding a method preserveOffset to format output so that they contain or not the CSV document record offset if needed. By default the CSV document offset is not given.
20d0eda to
e3a5819
Compare
e3a5819 to
e78eb9f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduction
This PR introduces the
League\Csvv9 public API. This version goals is to make the package more SOLID.Current API design issues:
The Reader class
When extracting data from a CSV document you are required to issue multiple times the same queries because the query/filtering methods are coupled directly to the Reader class. In doing so we create
The Writer class
Writer::insertOnedoes nothing on insertion error. Currently the errors are silently ignored.Writer::insertOneacceptsarrayandstring. If a string is passed it will be converted into a array usingstr_getcsv. This behaviour is problematic at best. As it may lead to incorrect insertion.Writerclass does many things it should not be able to do regardingreadinga CSV.Improve Stream filtering
in the v8 line we introduced support for PHP's stream. In v9, we can fully take advantage of that to:
Proposal
This PR:
Introduces two new classes by decoupling the
Readerclass into :Statementclass to ease selecting records from the CSV data. This class is immutable.RecordSetclass to manipulate the resulting records.Improves the
Readerclass which can now properly handle the CSV header and strip automatically the BOM sequence. Because of the decoupling, extracting methods and filtering methods are also improved and simplified.Improves the
Writerclass by:Readerclass)the AbstractCsv public API
the Writer public API extends the AbstractCsv public API
the Reader public API extends the AbstractCsv public API
Implements the
IteratorAggregateinterfaceThe Statement public API
The RecordSet public API
Implements the following interface
Countable,IteratorAggregate,JsonSerializebefore:
after:
Backward Incompatible Changes
Writeclass no longer implements the following interfacesJsonSerializable,IteratorAggregateAbstractCsv.newWriter,newReader.Statementclass).fetchDelimitersOccurrence(attached to theReaderclass).setNewline,getNewline(attached to theWriterclass).setInputEncoding(renamedsetConversionInputEncodingand attached to theRecordSetclass).Readerclass (attached to theRecordSetclass).callableparameter from the extract methods is removedWriter::insert*methods now throws exception on error and returns the number of bytes added.Targeted release version
version 9.0.0
Open issues
Since this is a major release some BC breaks are introduced. Some of them can be kept to preserve some of the current behavior:
Readerclass with the introduction of the newReader::selectmethod. Question: Should we use aliasing to keep them and mark them as deprecated for v9 and completely remove them for the next version or not ?