What's the utility of defining the "Error" exception. Why not use an existing one, say InvalidOperationException, or a plain Exception. Is making your own better practice?
There is no utility. It's perhaps written for JavaScript developers who are used to Error.. but it's not idiomatic C#. Might be indicative of a copilot too.
The use of a class-scoped `StringBuilder` that only one method uses, and `ReadQuotedColumn`/`ReadNonQuotedColumn` yielding one character at a time, rather than accepting a the builder isn't a good sign either (for efficiency). Or casting everything to a `char` (this won't support UTF8), or assuming an end quote followed by anything (:71) is valid way to end a field.
I will add a micro benchmark to see if the `yield return` is slowing things down, compared to just calling _sb.Add() inside Read*(). I will also see if it looks cleaner that way. To be honest, the `yield return` is currently in there just because I thought it's "cool".
It's good practice to throw an exception from your own namespace if you're writing a library.
You don't want to expose an implementation detail like some specific exception as part of your public API and have to worry about breaking that later.
You could overload some built in exception but IMO that's not the best practice. You muddy your API and a caller has to wrap your exception if they want to bubble it up and catch it specifically, anyway.
gnabgib|1 year ago
The use of a class-scoped `StringBuilder` that only one method uses, and `ReadQuotedColumn`/`ReadNonQuotedColumn` yielding one character at a time, rather than accepting a the builder isn't a good sign either (for efficiency). Or casting everything to a `char` (this won't support UTF8), or assuming an end quote followed by anything (:71) is valid way to end a field.
neonsunset|1 year ago
Having StringBuilder be a private field on the parser instance is not an issue either - it is simply reused.
vilark|1 year ago
vilark|1 year ago
jayd16|1 year ago
You don't want to expose an implementation detail like some specific exception as part of your public API and have to worry about breaking that later.
You could overload some built in exception but IMO that's not the best practice. You muddy your API and a caller has to wrap your exception if they want to bubble it up and catch it specifically, anyway.
taspeotis|1 year ago
It is forbidden.
https://learn.microsoft.com/en-us/dotnet/standard/exceptions...
> Exception ... None (use a derived class of this exception).
https://learn.microsoft.com/en-us/dotnet/standard/design-gui...
> DO NOT throw System.Exception or System.SystemException.
throwaway2990|1 year ago
recursive|1 year ago