I think that ReadHeader method name is confusing. The expectation is that ReadHeader will read the new record , not parse the previously read record.
The code
csv.Read();
csv.ReadHeader();
can be interpreted as "read the first line and then read the second line and consider the second line as a header".
I understand that changing the name to e.g. ParseHeader will be a breaking change, but you may consider to introduce new method and make existing obsolete.
The call at the beginning after creating CsvReader(textReader);
csv.ReadHeader();
throws 'No header record was found.' exception,聽 which was not clear, until I found in Google https://github.com/JoshClose/CsvHelper/issues/808 'No header record was found.'
I suggest to change
https://github.com/JoshClose/CsvHelper/blob/95d16775abeed7ce8136f44e002667a7c571387b/src/CsvHelper/CsvReader.cs
if( context.HeaderRecord == null )
instead of throwing exception call Read() inside the method and then call
ParseNamedIndexes();
It will avoid throwing exception and make the code more intuitive as allow to call ReadHeader() without preliminary Read()
I had the same issue. I wanted to check for specific field names and if the header existed at all for handling and ran into some walls. Also using PrepareHeaderForMatch to normalize files when using GetRecords seemed I got mixed results when mixing the Read(), ReadHeader() vs, just calling PrepareHeaderForMatch before the GetRecords alone. Some strange stuff going on but workable.
It would be nice of the simple Has Header flag can be used to seperate validation steps without having to read.
I had the same issue. Please, consider this change.
Furthermore, this isn't explain in the documentation how to read header. To understand, I went to see in the sources.
Edit :
It is explain in the documentation, my bad :
https://joshclose.github.io/CsvHelper/reading/#reading-records
I'm going to look into having it just do the first read if it hasn't already happened.
What if the file has multiple sections with multiple headers. Calling ReadHeader as the first action in the file would also call Read, but doing it later in the file would not. Would that make it confusing?
For new users, just by looking at the name, ReadHeader without actual read is confusing.
This is why I suggested to rename the current ReadHeader to ParseHeader or ParseRecordAsHeader that describes the actual action, and (new) ReadHeader will call ParseRecordAsHeader and then try to read a new record only if context.HeaderRecord is null.
Is it better to try recover from error instead of throw exception?
Unfortunately as your methods are virtual, it鈥檚 hard to predict, what behaviour derived classes are expected, so it will be a breaking change.
a good reason for this functionality at least for me is to get a list of headers so that I can validate whether or not the data has would it should have before I do the data-intensive parse or read. I worked around it it's not that big of a deal but it would be nice to have some clarity on this.
It may be also worth considering a new Configuration option like "AutomaticallyReadFirstLineAsHeader" or something that causes the Reader to call Read() and ReadHeader() implicitly.
I know you had good reasons for changing the code in the past to not always implicitly read headers, but the restoration of that ability locked behind a Configuration option that defaults to 'false' would likely benefit a substantial amount of users.
@MMOSimca, imho, the change will be beneficial only for NEW users, who don鈥檛 know, that ReadHeader doesn鈥檛 read anything. Adding configuration flag, defaulted to the existing behavior, will not help them, because they will not know that they should change something.
This just caught me too, went with just ReadHeader() then got told about no header.
Same thing for me - quite confusing to have a ReadHeader method that does not actually read anything!
As a non-breaking change you can create a new method
ReadAndParseHeader()
{
csv.Read();
csv.ReadHeader();
}
And in the documentation/samples refer to the new method.
ReadHeader will be marked as obsolete, but kept in code for backward compatibility.
Most helpful comment
This just caught me too, went with just ReadHeader() then got told about no header.