mperdeck / LINQtoCSV

Popular, easy to use library to read and write CSV files.
199 stars 112 forks source link

Migration to NETStandard 2.0 #62

Open gizmohd opened 6 years ago

gizmohd commented 6 years ago

Migration of codebase to NETStandard 2.0. With some efficency improvements made for file reading and writing to take advantage of the framework

mperdeck commented 6 years ago

Thanks for your pull request. Apologies for the delay.

I noticed that in class CsvContext, there is no #if NETSTANDARD to define an async version of the ReadData method, even though in the body there is #if NETSTANDARD to await an async method.

To be honest, I'm not a big fan of coupling the framework (net45 or NETSTANDARD) to the use of async methods. On the other hand, I can see the benefits of async methods. But a lot of code out there is not async and would need a lot of work to make async. So that is a bit of a dilemma.

You may have noticed that I've been neglecting this project for a long time. But your pull request got me thinking of introducing a version 2 which would have these breaking changes:

gizmohd commented 6 years ago

I made the changes because I needed to use the functionality in an aspnet core application....

By adding the async methods when reading a large csv file helped to ensure that

  1. Operations were not in the main thread
  2. Sped up the reading of the file (especially larger ones)

Most users who use netstandard are well versed in async operations. While it’s not used as much in classic sorbet. That’s why I made the project compile both. So in the nuget, you will have two binaries.

  1. Classic .net w/o the async operations
  2. Netstandard that has it.

So existing users wouldn’t have a breaking change.

Sent from my iPhone

On May 26, 2018, at 10:28 AM, mperdeck notifications@github.com wrote:

Thanks for your pull request. Apologies for the delay.

I noticed that in class CsvContext, there is no #if NETSTANDARD to define an async version of the ReadData method, even though in the body there is #if NETSTANDARD to await an async method.

To be honest, I'm not a big fan of coupling the framework (net45 or NETSTANDARD) to the use of async methods. On the other hand, I can see the benefits of async methods. But a lot of code out there is not async and would need a lot of work to make async. So that is a bit of a dilemma.

You may have noticed that I've been neglecting this project for a long time. But your pull request got me thinking of introducing a version 2 which would have these breaking changes:

minimum framework is net45 (dropping net35) or NETSTANDARD; all methods are async; replacing column attributes with counterparts from the System.ComponentModel.DataAnnotations namespace; its own site with Github Pages. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.