Remove AsList AsDT and replace them with Extension Methods

[MarcosMeli]

That will reduce the API surface of the library and clean the types

[jboyer2012]

I'm going to try my hand at this as my first OSS contribution! Please be kind to my pull request.

Is this referring to the ReadFileAsList methods that I see in FileHelperEngine.cs? If so, what are your thoughts as to what type the methods would be extending (FileHelperEngine, InternalStreamReader, the files themselves)? Also, have you given any thought to how the API would be used with the extension methods?

My thought is this:

var engine = new FileHelperEngine(recordType);
var list = engine.AsList(file1);

The only issue I see here is that you want the file as a list and not the engine even though this makes sense based on where the current code resides. However, it may feel unwieldy to the end user.

[MarcosMeli]

Hi @jboyer2012 thanks for join the team !!

My fault for don't explain better the idea.

At this time, when you create an engine, and do

 engine.

The intellisense will show ReadFileAsDT ReadFileAsList ReadStreamAsDT ReadStreamAsList ReadStringAsDT readStringAsList

All These methods needs to be removed for 4.0 version and we will provide only

ReadFile ReadStream ReadString

Later the user can call an extension method in the resulting array to get what he wants

var result = engine.ReadFile();
var resultAsList = result.ToList() // The Array already has an extension method in linq to create a List
var resultAsDT = result.ToDataTable() // This is the extension method you need to write, you recieve an Array and return a DataTable

You can check http://www.filehelpers.net/docs/html/M_FileHelpers_CommonEngine_RecordsToDataTable.htm

For a start point, from an old implementation

Thanks

[jboyer2012]

Hi Marcos,

I understand better now. Does this project have an existing namespace for extension methods or will I have to create a new one?

[MarcosMeli]

@jboyer2012 At this time because the library is .net 2.0 compatible, there are no extension methods, I suffer it a lot, but no more, 3.1 will be the last version of the library to support .net 2.0

About the namespace, please use FileHelpers to allow better discovery and avoid users to import more namespaces

Thanks !!

[jboyer2012]

Okay, I'll do that. Another question from a design standpoint, since I'm still wrapping my head around how you guys have built the code. I am looking at the RecordsToDataTable method of the RecordOperations class as an example. I notice though a call to CreateEmptyDataTable which uses an IRecordInfo to create the DataTable to populate. Will I need to get an IRecordInfo from the Array (or somewhere else) before I can transform it into a DataTable? How would I receive this in my method, as a parameter perhaps?

[MarcosMeli]

@jboyer2012 No problem at all take your time, the core of the library can be a bit old because no linq, no too much generics inside.

You must create a record info via the
https://github.com/MarcosMeli/FileHelpers/blob/master/FileHelpers/Core/RecordInfo.Factory.cs

 RecordInfo.RecordInfoFactory.Resolve(Type of the array)

So you get an IRecordinfo

The signature of the method must be:

public static DataTable ToDataTable<T>(this T[] records)
{
      var ri = RecordInfo.RecordInfoFactory.Resolve(typeof(T));
        ...
}

Cheers

[jboyer2012]

@MarcosMeli I sent a pull request your way for the new extension method. I began to eliminate ReadFileAsDT from the code so that the API is cleaner but I noticed the CsvEngine inherits from FileHelperEngine. Therefore removing the method from FIleHelperEngine breaks Csv Engine. Since you didn't mention CsvEngine or it's DT methods previously I wasn't sure what you wanted to do with these. Are these staying? I'm assuming the DT methods will be removed from FileHelperEngine and CommonEngine. Let me know if this is correct.

[jboyer2012]

Also I added a bit of a hack (which is commented) due to the fact that we are building to target .NET 2.0 in the project. This was to make the "this" keyword in the extension method signature build successfully. I don't like it being there but I didn't want to switch the target framework without consulting you first. So I added the extra code in the FileHelperExtensions file so that the functionality will work and you can decide what to do from there. It added a bunch of warnings and failed the CI build. Not sure what you wanted to do with this issue.

[MarcosMeli]

@jboyer2012 Thanks a ton !!

Just merged, I will remove the this hack attribute (because I will change it to .net 4.0), and maybe we can mark the methods as [Obsolete] for the next version and remove them later.

Now we have the Analyzer http://www.filehelpers.net/analyzer/ so we can create a rewrite that change ReadAsList() and ReadAsDT to the extension methods

Thanks !!