CsvReader.CsvReaderBuilder#build handle BOM configuration one level deeper?

[archiewestermann]

First of all ... THANK you for this excellent fast and clean and well documented library (without further dependencies).

BOM detection currently is handled on the CsvReader.CsvReaderBuilder#build method that accepts a java.nio.Path

This method delegates down to the build method that accepts a Reader.

Sometimes (e.g. if you are loading test resources via stream, or download byte[] from aws S3) you don't have a file on a file system.
You then would create some Reader and put it to the latter build method.
In that case, even if you configured "detectBomHeader(true)" for the builder, it will be without effect.

We are currently fixing this by:

And we are doing the BOM detection via stream decorator pattern

We have to use commons-io for this.
Changing BomUtil to be a Reader with decorator capability would be great.

cheers, Achim

[osiegmar]

Thanks for your feedback and for bringing up this idea!

In that case, even if you configured "detectBomHeader(true)" for the builder, it will be without effect.

That's correct. As written in the Javadoc of detectBomHeader(boolean):

BOM detection only applies for direct file access.

The reason for this is that FastCSV currently provides terminal operations for three different types of input sources: java.nio.file.Path, java.lang.String and java.io.Reader.

So, only the first one provides binary access to the data as String and Reader are already decoded.

To extend the automatic BOM detection, additional terminal operations would be required (such as for java.io.InputStream).

Changing BomUtil to be a Reader with decorator capability would be great.

My first (pre-release) implementation did exactly that. However, the performance sucked big time compared to the current implementation. The decorated reader has to perform an extra check for the BOM-detection-status on every single read operation.

I'll consider this again.

[archiewestermann]

Thank you a lot, this was very quick.
Good solution, BomInputStream does not require extra check for each read due to constructor.
Our team made the right choice using your library.

[osiegmar]

Continued in #130