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.

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

Add Interfaces to allowed creation of Factory of Mappers #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

felpasl
Copy link

@felpasl felpasl commented Apr 11, 2019

Change Mapping to Interface to make a Library open to Factory Pattern,

Fix Enum Convert, based on Char values.

@bytefish
Copy link
Collaborator

This is a rather large Pull Request. Please give me some time for a review. ✍️

@bytefish
Copy link
Collaborator

Could you please describe the reason for the changes? Do you need to mock the CsvMapping for tests? I saw you also changed the EnumConverter, why not implement a different converter if there is a need for a different behavior?

@felpasl
Copy link
Author

felpasl commented Apr 13, 2019

1 - for the first change, Im using the factory pattern to read an entity from file using generics, with interface we are able to do this:

    public class LoadDataFromFile
    {
        CsvParserOptions csvParserOptions;
        string basePath;

        public LoadDataFromFile(string basePath)
        {
            csvParserOptions = new CsvParserOptions(false, '@');
            this.basePath = basePath;
        }

        public List<T> ReadFile<T>() where T : class, new()
        {
            ICsvMapping<T> mapping = Mapping.MapFactory.Create<T>();
            CsvParser<T> csvParser = new CsvParser<T>(csvParserOptions, mapping);

            List<CsvMappingResult<T>> result = new List<CsvMappingResult<T>>();
            DirectoryInfo d = new DirectoryInfo(basePath);
            var files = d.GetFiles($"{GetFileName(mapping)}").ToList();

            foreach (FileInfo file in files)
            {
                result.AddRange(csvParser.ReadFromFile(file.FullName, Encoding.GetEncoding("ISO-8859-1")).ToList());
            }            

            if (result.Any(r => r.IsValid == false))
                throw new InvalidCastException("Error on reading DNE :" + string.Join("|", result.Where(r => r.IsValid == false)
                    .Select(r => $"row {r.RowIndex}, {r.Error}").ToList()));

            return result.Select(r => r.Result).ToList();
        }

        public string GetFileName(object mapping)
        {
            var fnAttribute = mapping.GetType().GetCustomAttributes(
                typeof(DneFileNameAttribute), true
            ).FirstOrDefault() as DneFileNameAttribute;
            if (fnAttribute != null)
            {
                return fnAttribute.FileName;
            }
            return null;
        }
    }

this is a work in progress code.

2 - From Enum, we have return like 'C', 'D' from file, and Enum with City and District with value 'C' and 'D', this is not unusual for enum types, and I thought it could be an improvement.

@felpasl felpasl changed the title Add Interfaces to allowed creation of Factoring of Mappers Add Interfaces to allowed creation of Factory of Mappers Apr 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants