Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support for Immutable types #85

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Support for Immutable types #85

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
82fd156
Replace constructor with System.Activator.CreateInstance
JoaoSeverino May 20, 2022
2bc49db
Minor refactor
JoaoSeverino May 20, 2022
5bc1d87
Support for immutable types and map to constructor parameter
JoaoSeverino May 20, 2022
d96d345
Add Visual Studio gitignore
JoaoSeverino May 21, 2022
82af36c
Support for immutable types with MapUsing style syntax
JoaoSeverino May 21, 2022
f0a6af1
Add Unit Tests for MapConstructorParameter
JoaoSeverino May 21, 2022
0db13e5
Add protection in Activator and Unit Tests for MapUsing
JoaoSeverino May 21, 2022
07af7f8
Merge branch 'TinyCsvParser:master' into Issue84_ImmutableTypes
JoaoSeverino Jun 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion TinyCsvParser/TinyCsvParser/Mapping/CsvMapping.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,10 @@ private void AddPropertyMapping<TProperty>(RangeDefinition range, CsvCollectionP

public CsvMappingResult<TEntity> Map(TokenizedRow values)
{
TEntity entity = new TEntity();
List<object> args = new List<object>();
// TODO build constructor arguments

TEntity entity = (TEntity)Activator.CreateInstance(typeof(TEntity), args.ToArray());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better to pass the object creator in the constructor. Right now the constructor only contains the ITypeConverterProvider typeConverterProvider, but we could also add a Func<TEntity> lambda to create the target object.

But honestly, looking at it the CsvMapping looks way too convoluted, we can simplify it a lot I think.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, it would be nice to be able to pass the object creation logic to the CsvMapping constructor
Do you propose something like a Func<TokenizedRow, TEntity> lambda like the one in the new MapUsing method and replace the list with a single variable filled in from the constructor? Or how do you see it?

protected CsvRowConstructor<TEntity> MapUsing(Func<TokenizedRow, TEntity> action)
{
	var rowConstructor = new CsvRowConstructor<TEntity>(action);

	csvUsingConstructorMappings.Add(rowConstructor);
	
	return rowConstructor;
}


// Iterate over Index Mappings:
for (int pos = 0; pos < csvIndexPropertyMappings.Count; pos++)
Expand Down