The extraction of shared serialization logic makes sense, too. It introduces potential variance between the CSV's data and the serializer, because the serializer can be ab-used independently (see below). But in principle, it's a good idea IMHO!
However, would you have any concerns about making Serializer be public? It might help me with a project I'm working on.
(Actually, what I really want is to add additional rows to an existing file. I'm still working out the design, so it's not clear yet whether I should append rows to my DataView and serialize the whole thing, or go directly to the file APIs to write out only the new rows.)
In my Mac app TableFlip, I import from CSV to an internal tabular data structure, and export that to various formats.
I could see how the CSV type could be useful as a mutable table representation, but that's a bit out of scope for CSV -- that's a more generic "table" or a "tabular". (We could add that, too, but that needs more discussion I think.)
Producing CSV export data here does still make total sense, though, e.g. to quickly convert delimiters. Mutating the CSV output a bit sounds like a CSV-centric use case during export, too. It does make sense.
In the context of the SwiftCSV module, the name Serializer fits well. But when it becomes public API, should we call it CSVSerializer instead? Then again SwiftCSV.Serializer can be used to disambiguate, too. I have no strong opinion on this.
When we make Serializer public, we need handle more inconsistent data: it used to be the CSV views's job to assemble the serializable content in a way that made sense to the CSV view.
But if the serializer is extracted, and esp. if it's public, we can't make any assumptions.
We'd need
unit tests for all public functions, e.g. to demonstrate that the enquoting of cell contents works as expected;
handle non-rectangular data, i.e.:
what happens if some rows have less/more elements than others?
what happens if the header cell count is x, but a row's element count is >x?
I tend towards making header.count canonical; cut off row cells at offsets beyond header.count, and if rows are too short, fill with empty cells -- so that in effect, each row has the same amount of delimiters
This is a bit of extra work. Do you think that's worth it for your use case? If so, let's check out how to design this properly 💪
On 2022-08-25, @DivineDominion wrote:
The extraction of shared serialization logic makes sense, too. It introduces potential variance between the CSV's data and the serializer, because the serializer can be ab-used independently (see below). But in principle, it's a good idea IMHO!
In my Mac app TableFlip, I import from CSV to an internal tabular data structure, and export that to various formats.
I could see how the CSV type could be useful as a mutable table representation, but that's a bit out of scope for CSV -- that's a more generic "table" or a "tabular". (We could add that, too, but that needs more discussion I think.)
Producing CSV export data here does still make total sense, though, e.g. to quickly convert delimiters. Mutating the CSV output a bit sounds like a CSV-centric use case during export, too. It does make sense.
In the context of the SwiftCSV module, the name
Serializerfits well. But when it becomes public API, should we call itCSVSerializerinstead? Then againSwiftCSV.Serializercan be used to disambiguate, too. I have no strong opinion on this.When we make
Serializerpublic, we need handle more inconsistent data: it used to be the CSV views's job to assemble the serializable content in a way that made sense to the CSV view.But if the serializer is extracted, and esp. if it's public, we can't make any assumptions.
We'd need
x, but a row's element count is>x?header.countcanonical; cut off row cells at offsets beyondheader.count, and if rows are too short, fill with empty cells -- so that in effect, each row has the same amount of delimitersThis is a bit of extra work. Do you think that's worth it for your use case? If so, let's check out how to design this properly 💪
Originally posted by @DivineDominion in https://github.com/swiftcsv/SwiftCSV/pull/107#pullrequestreview-1085875931