6
\$\begingroup\$

I am initializing the following object from a CSV file:

private class MassUploadUser
    {
        public string email { get; set; }

        public string firstName { get; set; }

        public string lastName { get; set; }

        public string role { get; set; }

        public bool active { get { return true; } }
    }

I later serialize this to JSON using Newtonsoft, and then Post this to a RESTful API. TextFieldParser is from Microsoft.VisualBasic.FileIO by the way.

using (var parser = new TextFieldParser(Program.file))
{
    parser.SetDelimiters(",");

    string[] header = parser.ReadFields();

    while (!parser.EndOfData)
    {
            string[] fields = parser.ReadFields();

            var massUploadUser = new MassUploadUser();
            string password = null;

            for (int i = 0; i < fields.Length; i++)
            {
                switch (header[i])
                {
                    case "First Name":
                        massUploadUser.firstName = fields[i];
                        break;

                    case "Last Name":
                        massUploadUser.lastName = fields[i];
                        break;

                    case "Role":
                        massUploadUser.role = fields[i];
                        break;

                    case "email":
                        massUploadUser.email = fields[i];
                        break;

                    case "Password":
                        password = fields[i];
                        break;
                }
            }

           // After the for loop, I have some additional logic
           // to serialize the newly-created object to JSON
           // and then POST it to a RESTful API
           DoPostToWebService(massUploadUser, password);

          // At this point, we created the new user on the server, so no need
          // to keep the current instance of massUploadUser around
        }

Can this be improved? Is there a more "general" way to do this?

\$\endgroup\$
  • \$\begingroup\$ If the CSV file always has the same field order the switch is unneessary of course. \$\endgroup\$ – radarbob Mar 20 '18 at 23:18
  • \$\begingroup\$ Have you considered using the CsvHelper framework? \$\endgroup\$ – Nkosi Mar 21 '18 at 1:11
  • 1
    \$\begingroup\$ Can you give us a sample input file? \$\endgroup\$ – Dannnno Mar 27 '18 at 3:27
5
\$\begingroup\$

MassUploadUser

Based on the .NET Naming Guidelines properties should be named using PascalCase casing.

The loop

In its current form you do for each column in each row a switch to get the column index and value. By extracting this to a separate method like so

private  Dictionary<string, int> ReadColumnIndexes(string[] headers)
{

    return headers.Select((v, i) => new { Key = v, Value = i })
                  .ToDictionary(o => o.Key, o => o.Value);
}  

You can then, outside of the while loop, assign the desired column index to variables like so

var columnDictionary = ReadColumnIndexes(headers);

var firstNameColumn = columnDictionary["First Name"];
var lastNameColumn = columnDictionary["Last Name"];
.....  
var passwordColumn = columnDictionary["Password"];

now your loop could look like so

while (!parser.EndOfData)
{
    string[] fields = parser.ReadFields();

    var massUploadUser = new MassUploadUser();
    massUploadUser.firstname = fields[firstNameColumn];
    massUploadUser.lastname = fields[lastNameColumn];
    .....
    string password = fields[passwordColumn];

   // After the for loop, I have some additional logic
   // to serialize the newly-created object to JSON
   // and then POST it to a RESTful API
   DoPostToWebService(massUploadUser, password);

  // At this point, we created the new user on the server, so no need
  // to keep the current instance of massUploadUser around
}
\$\endgroup\$
6
\$\begingroup\$

Here is another answer, using Reflection, Expression and Attribute to create a more automated solution.

First you create a couple of custom Attributes to mark fields or properties with the CSV Header Name when it doesn't match the class member name, or to indicate a class member isn't initialized from the CSV:

[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false)]
public class CSVColumnName : Attribute {
    public string ColumnName { get; }
    public CSVColumnName(string name) => ColumnName = name;
}

[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false)]
public class CSVNoColumnName : Attribute {
}

Then you can create a static method to build the PropertySetters Dictionary for you by Reflection. You need a couple of extension methods to make MemberInfo easier to work with:

public static Type GetMemberType(this MemberInfo member) {
    switch (member.MemberType) {
        case MemberTypes.Field:
            return ((FieldInfo)member).FieldType;
        case MemberTypes.Property:
            return ((PropertyInfo)member).PropertyType;
        case MemberTypes.Event:
            return ((EventInfo)member).EventHandlerType;
        default:
            throw new ArgumentException("MemberInfo must be if type FieldInfo, PropertyInfo or EventInfo", "member");
    }
}

public static bool GetCanWrite(this MemberInfo member) {
    switch (member.MemberType) {
        case MemberTypes.Field:
            return true;
        case MemberTypes.Property:
            return ((PropertyInfo)member).CanWrite;
        default:
            throw new ArgumentException("MemberInfo must be if type FieldInfo or PropertyInfo", "member");
    }
}

Then you can create the setter lambdas for each field and put them in a Dictionary:

public static class CSVMapping<T> {
    public static Dictionary<string, Action<T, object>> PropertySetters() {
        var t = typeof(T);
        var propsOrFields = t.GetMembers(BindingFlags.Instance | BindingFlags.Public).Where(m => m.MemberType == MemberTypes.Property || m.MemberType == MemberTypes.Field);

        var ans = new Dictionary<string, Action<T, object>>(StringComparer.OrginalIgnoreCase);
        foreach (var m in propsOrFields) {
            if (!Attribute.IsDefined(m, typeof(CSVNoColumnName)) && m.GetCanWrite()) {
                var ca = (CSVColumnName)Attribute.GetCustomAttribute(m, typeof(CSVColumnName));
                var csvname = (ca != null) ? ca.ColumnName : m.Name;
                // (T p1, object p2) => p1.{m.Name} = ({m.Type})p2;
                var paramobj = Expression.Parameter(t);
                var paramval = Expression.Parameter(typeof(object));
                var body = Expression.Assign(Expression.PropertyOrField(paramobj, m.Name), Expression.Convert(paramval, m.GetMemberType()));
                var setter = Expression.Lambda<Action<T, object>>(body, new[] { paramobj, paramval });
                ans.Add(csvname, setter.Compile());
            }
        }
        return ans;
    }
}

Note: You could use reflection to assign values as well, but I figure it is worth the effort to build the lambdas since you presumably will be assigning to the fields often (once per CSV file row).

Now you can annotate the class with CSV information:

private class MassUploadUser {
    public string email { get; set; }

    [CSVColumnName("First Name")]
    public string firstName { get; set; }

    [CSVColumnName("Last Name")]
    public string lastName { get; set; }

    public string role { get; set; }

    [CSVNoColumnName]
    public bool active { get { return true; } }
}

Note: Since active is a read-only property, the PropertySetters method would skip it even if it didn't have the attribute.

Finally, you can convert the CSV file to class members just like my previous answer, using the setters Dictionary to assign read-in values:

var fieldMap = CSVMapping<MassUploadUser>.PropertySetters();

using (var parser = new TextFieldParser(csvFilename)) {
    parser.SetDelimiters(",");

    string[] header = parser.ReadFields();
    var headerMap = header.Select((h, i) => new { h, i }).ToDictionary(hi => hi.h, hi => hi.i);

    while (!parser.EndOfData) {
        string[] fields = parser.ReadFields();

        var massUploadUser = new MassUploadUser();
        foreach (var field in fieldMap.Keys)
            fieldMap[field](massUploadUser, fields[headerMap[field]]);
    }
}
\$\endgroup\$
  • \$\begingroup\$ You beat me to it. I am currently doing a lambda version as well but without the attributes. Got stuck on the conversion for non string properties but that was my own fault for trying to do too much. lol \$\endgroup\$ – Nkosi Mar 21 '18 at 19:32
  • \$\begingroup\$ How stuck? Expression.Convert handles most ills? \$\endgroup\$ – NetMage Mar 21 '18 at 19:38
  • \$\begingroup\$ Nope that is what I thought as well. But I keep getting No coercion operator is defined between types for example converting string to int and so forth. Been working on that little bug over an hour now. :) \$\endgroup\$ – Nkosi Mar 21 '18 at 19:40
  • 1
    \$\begingroup\$ Solutions with Expressions always deserve +1 ;-) nice, if you only could add the equivalent of the constructed expression in C# it would be easier to follow and visualize what you are actually building there. \$\endgroup\$ – t3chb0t Mar 22 '18 at 8:46
  • 1
    \$\begingroup\$ @t3chb0t I added a comment showing the lambda. \$\endgroup\$ – NetMage Mar 23 '18 at 17:52
4
\$\begingroup\$

Since you already knew the headers as used in your for loop, then a delegate would reduce the number of loops needed to do the repeated task of creating the POCO.

string[] headers = parser.ReadFields();
Func<string[], MassUploadUser> factory = (string[] fields) => 
    new MassUploadUser {
        email  = fields[headers.IndexOf("email")],
        firstName  = fields[headers.IndexOf("First Name")],
        lastName = fields[headers.IndexOf("Last Name")],
        role = fields[headers.IndexOf("Role")]
    };

//...

Which in itself is rather simple for this example. But for more complex data, can become really complicated really fast.

I am very partial to building expressions and delegates to handle generic repeated tasks that lend themselves well to LINQ. This scenario would fall into that category.

Building upon the already provided suggestions, which provide some good suggestions, a lambda expression can be built using reflection and naming conventions to simplify the creation of your objects from the CSV file.

The following utilities were constructed to satisfy that. Hopefully the code and accompanying documentation speaks for itself.

public static class ExpressionManager {

    /// <summary>
    /// Builds an expression that creates a new object and initializes properties from a string array
    /// </summary>
    public static Expression<Func<string[], T>> BuildFactoryExpressionFor<T>(string[] headers) where T : new() {
        var type = typeof(T);
        var properties = type.GetCachedProperties();
        var columns = MapColumnIndexes(headers);
        //Desired delegate
        //Func<string[], T> factory = (string[] fields) => new T() { X = fields[0], Y = fields[1] };

        // (string[] fields) => ...
        var parameter = Expression.Parameter(headers.GetType(), "fields");
        // new T()
        var newOfType = Expression.New(type);
        // { PropertyA = fields[0], PropertyB = (int)fields[1] }
        var memberBindings = getMemberBindings(columns, properties, parameter);
        // new T() { PropertyA = fields[0], PropertyB = (int)fields[1] }; 
        var body = Expression.MemberInit(newOfType, memberBindings);
        // (string[] fields) => new T() { PropertyA = fields[0], PropertyB = (int)fields[1] }; 
        var lambda = Expression.Lambda<Func<string[], T>>(body, parameter);
        return lambda;
    }
    /// <summary>
    /// Get the bindings used to populate the provided properties
    /// </summary>
    private static IEnumerable<MemberAssignment> getMemberBindings(IDictionary<string, int> columns, PropertyInfo[] properties, ParameterExpression parameter) {
        using (var e = columns.Keys.GetEnumerator()) {
            while (e.MoveNext()) {
                var headerName = e.Current;
                var propertyName = headerName.Replace(" ", "");//<-- simple naming convention
                var propertyInfo = properties.FirstOrDefault(_ => string.Equals(_.Name, propertyName, StringComparison.InvariantCultureIgnoreCase));
                if (propertyInfo != null) {
                    var setMthd = propertyInfo.GetSetMethod(true);
                    if (propertyInfo.CanWrite && setMthd != null && setMthd.IsPublic) {
                        var propertyType = propertyInfo.PropertyType;
                        // index
                        var headerIndex = Expression.Constant(columns[headerName]);
                        // fields[index]
                        Expression value = Expression.ArrayAccess(parameter, headerIndex);
                        if (propertyType != typeof(string)) {
                            // (int)Coerce(fields[index], typeof(int))
                            value = Expression.Convert(Expression.Call(getConverter(), value, Expression.Constant(propertyType)), propertyType);
                        }
                        // Property = value                            
                        var setter = Expression.Bind(propertyInfo, value);
                        yield return setter;
                    }
                }
            }
        }
    }

    static MethodInfo coerce;
    static MethodInfo getConverter() {
        if (coerce == null) {
            var flags = BindingFlags.Static | BindingFlags.NonPublic;
            coerce = typeof(ExpressionManager).GetMethod("CoerceValue", flags);
        }
        return coerce;
    }

    static object CoerceValue(object value, Type conversionType) {
        if (value == null || (value is string && string.IsNullOrWhiteSpace(value as string))) {
            return conversionType.GetDefaultValueForType();
        }
        //TODO: room for improvement here for other types. consider automapper.
        try {
            return Convert.ChangeType(value, conversionType,
                System.Globalization.CultureInfo.InvariantCulture);
        } catch { }
        if (isNullable(conversionType)) {
            try {
                var underlyingType = Nullable.GetUnderlyingType(conversionType);
                return Convert.ChangeType(value, underlyingType,
                    System.Globalization.CultureInfo.InvariantCulture);
            } catch { }
        }
        return conversionType.GetDefaultValueForType();
    }

    static bool isNullable(Type conversionType) {
        return conversionType.IsGenericType &&
               conversionType.GetGenericTypeDefinition().IsAssignableFrom(typeof(Nullable<>));
    }

    static Dictionary<Type, object> defaultValueTypes = new Dictionary<Type, object>();
    /// <summary>
    /// Gets the default value for a type.
    /// </summary>
    /// <param name="type">The type.</param>
    /// <returns>The default value.</returns>
    static object GetDefaultValueForType(this Type type) {
        if (!type.IsValueType) return null;

        object defaultValue;

        if (defaultValueTypes.TryGetValue(type, out defaultValue)) return defaultValue;

        defaultValue = type.CreateInstance();

        defaultValueTypes[type] = defaultValue;

        return defaultValue;
    }

    public static IDictionary<string, int> MapColumnIndexes(this string[] headers) {
        return headers
            .Select((header, index) => new { header, index })
            .ToDictionary(o => o.header, o => o.index);
    }

    private static readonly IDictionary<Type, PropertyInfo[]> propertyCache = new Dictionary<Type, PropertyInfo[]>();
    /// <summary>
    /// Returns all the public properties of the current <seealso cref="System.Type"/>.
    /// </summary>
    /// <param name="type">The type to get the properties from</param>
    /// <returns></returns>
    public static PropertyInfo[] GetCachedProperties(this Type type) {
        PropertyInfo[] properties = new PropertyInfo[0];
        if (!propertyCache.TryGetValue(type, out properties)) {
            lock (propertyCache) {
                if (!propertyCache.TryGetValue(type, out properties)) {
                    var flags = BindingFlags.Public | BindingFlags.Instance;
                    properties = type.GetProperties(flags);
                    propertyCache[type] = properties;
                }
            }
        }
        return properties;
    }
}

My initial issue was with allowing for a more flexible factory delegate that would try to convert the provided string values when mapping to non-string properties.

//...

if (propertyType != typeof(string)) {
    // (int)Coerce(fields[index], typeof(int))
    value = Expression.Convert(Expression.Call(getConverter(), value, Expression.Constant(propertyType)), propertyType);
}

//...

This would allow a property like

public DateTime BirthDate { get; set; }

to map to a CSV column like

"2018-03-21"

and a proper DateTime would be passed to the mapped property.

For now, the simplified CoerceValue method should be able to handle simple type conversions between value types and also nullables. There is room for improvements here as a library like Automapper could come in handy.

Using a simple naming convention like removing spaces from CSV header names to compare against target property names, simplifies a one to one mapping of CSV header names to property names. Again this could be improved to use metadata from attributes if so desired as suggested in another answer.

for example

"First Name" => firstName
"Last Name" => lastName
...etc

While the provided utilities may look like a lot under the hood, it will allow for a more simplified method when refactored

using (var parser = new TextFieldParser(Program.file)) {
    parser.SetDelimiters(",");

    string[] headers = parser.ReadFields();
    //delegate to build desired objects
    var factory = ExpressionManager.BuildFactoryExpressionFor<MassUploadUser>(headers).Compile();
    //Need this to access Password as it is not included in POCO
    var passwordColumn = headers.IndexOf("Password");

    while (!parser.EndOfData){
        string[] fields = parser.ReadFields();
        MassUploadUser massUploadUser = factory(fields);
        string password = fields[passwordColumn];

        // After the for loop, I have some additional logic
        // to serialize the newly-created object to JSON
        // and then POST it to a RESTful API
        DoPostToWebService(massUploadUser, password);

        // At this point, we created the new user on the server, so no need
        // to keep the current instance of massUploadUser around
    }
}
\$\endgroup\$
  • \$\begingroup\$ hi, does this create C# objects/classes at runtime from CSV files. In my case, I dont know the headers prior, so I have to discover and create this at runtime. Will this help me.. & does this create the func needed, if (propertyType != typeof(string)) { // (int)Coerce(fields[index], typeof(int)) value = Expression.Convert(Expression.Call(getConverter(), value, Expression.Constant(propertyType)), propertyType); } \$\endgroup\$ – transformer Nov 8 '18 at 20:13
3
\$\begingroup\$

I am not sure if Reflection wouldn't be the ultimate solution (you would need to use a custom attribute to indicate non-matching CSV headers), but one possibility is to manually create a map for column names to fields in the MassUploadUser class:

public readonly static Dictionary<string, Action<MassUploadUser, object>> PropertySetters = new Dictionary<string, Action<MassUploadUser, object>>() {
    { "First Name", (u,v) => u.firstName = (string)v },
    { "Last Name", (u,v) => u.lastName = (string)v },
    { "Role", (u,v) => u.role = (string)v },
    { "email", (u,v) => u.email = (string)v },
};

Then you can create another map to map header names to column (field) numbers and process the mapped fields:

var fieldMap = MassUploadUser.PropertySetters;

using (var parser = new TextFieldParser(@"")) {
    parser.SetDelimiters(",");

    string[] header = parser.ReadFields();
    var headerMap = header.Select((h, i) => new { h, i }).ToDictionary(hi => hi.h, hi => hi.i);

    while (!parser.EndOfData) {
        string[] fields = parser.ReadFields();

        var massUploadUser = new MassUploadUser();
        foreach (var field in fieldMap.Keys)
            fieldMap[field](massUploadUser, fields[headerMap[field]]);
    }
\$\endgroup\$
0
\$\begingroup\$

I would suggest there are better ways than using the Microsoft.VisualBasic libraries. This isn't just because you are using C#; I have given the same advice many times in the past to those using VB.NET.

There are a lot of links on how to read text with .NET. Here is one using System.IO:

https://docs.microsoft.com/en-us/dotnet/standard/io/how-to-read-text-from-a-file

To split the fields, there is the String.Split method.

For reading the specific fields, see the answer from @Heslacher. Pay attention to his other tips such as naming, etc. Speaking of naming, MassUploadUser is not a very good name. I would suggest something simpler and more direct, such as User, or Person, or Employee.

I note you are reading one line at a time, and posting one line at a time. You may consider doing this in bulk, in which case you would want a List<User> (or List<MassUploadUser>).

\$\endgroup\$
  • 2
    \$\begingroup\$ Note that TextFieldParser handles reading CSV files with quote enclosed fields, a non-trivial subject that the article doesn't begin to cover. \$\endgroup\$ – NetMage Mar 21 '18 at 19:42
0
\$\begingroup\$

Single Responsibility

Your method does 2 things: (1) parse entities from CSV and (2) post entities over a web service. You should create a dedicated method to do the parsing. This improves (re-)usability and testability of your code.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.