Menu

Source Merge Request #10: CSVReaderHeaderAware (merged)

Merging...

Merged

Something went wrong. Please, merge manually

Checking if merge is possible...

Something went wrong. Please, merge manually

Andoreh wants to merge 0 commits from /u/alrosot/opencsv/ to master, 2017-12-04

I do believe that using @CsvBIndByName is the proper way to retrieve properties based on the header names.
But I can also see opportunity for this header aware reader when the resolution by name is desired but there's no appetite enough to create a special bean to be mapped to the CSV.

The reader I'm proposing has two main methods:
* readNext(String headerName): when you are interested in only one value of the line
* readMap(): which returns a mapping of headers and row values (Map<string,string>) that can be easily used on the application to navigate across fields by header name.</string,string>

If those suggestions are adopted, I'd be happy to propose another extension that would behave like a SQL projection. Something like:
readNext(String ... headers), and that would return a String[] of variable size depending on the columns the user requested.

This merge request was coded using TDD and the related tests are attached.

Commit Date  

Discussion

  • Scott Conway

    Scott Conway - 2017-12-04

    Thank you for your addition to openCSV - I like the extension model you used. The only thing I am going to change offhand is that in the skip method you added to CSVReader I am going to remove the try/catch and allow exceptions to come through. This is because we added the multi-line limit to protect against malformed csv files recently and your method will ignore the exception it throws.

     
  • Scott Conway

    Scott Conway - 2017-12-04
    • Status: open --> merged
     
  • Andoreh

    Andoreh - 2017-12-04

    My pleasure, Scott. Thanks for maintaining the project. The merge you accepted will be already useful for my company.
    Soon I will submit that second suggestion I mentioned above (readNext taking varargs). The impact  API-wise would be minimal.
    Cheers, Andre.

     
  • Scott Conway

    Scott Conway - 2017-12-04

    Well I did a few more changes than I originally stated.

    The big change was that if the number of data items read was not the same as the number of header columns then it will now throw an IOException. This goes along with a change I did for 4.1 because of several bug request I got about the HeaderColumnNameMappingStrategy when the data was invalid (but really because there were too many or too few columns). Now you would think that too many columns we should just ignore the extra and too few just return null (which we did for a long time) but when you think about it if you create a data file with a header row with three headers you should expect that each line has three values to match the columns. If not then there is either a typo which would shift the values of the data either into the next column or next record.

    The next change is that I added a null check to the readNext as well to prevent a NullPointerException when you skip past the end of file. This way the readMap and readNext have the same behavior.

    I also added Javadocs. This is used to generate our documentation, you can see the current documentation at http://opencsv.sourceforge.net/apidocs/index.html, so I consider that very important. Please check the javadocs I added to see that they are correct.

    With the exception of the javadocs I added tests for all the above changes. They all pass and have been merged in.

    If I get a chance tomorrow I will do a mvn site:site and see if findbugs commplains.

    Please check out the latest code to review the changes.

    Kind regards.

    Scott :)

     
  • Andoreh

    Andoreh - 2017-12-04

    Thanks, Scott. I believe they were all sensible adjustments. Apologies for the javadoc oversight.

     

Log in to post a comment.