HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Getting column values from a datatable

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
columndatatablegettingvaluesfrom

Problem

Is there a way I can make the code compact and with fewer ifs?

theData = GetData();
if (theData.Rows.Count > 0)
{
    MyModel = new CustomModel();
    dataSetRow = theData.Rows[0];
    if (theData.Columns.Contains("Column1"))
    {
        if ((!object.ReferenceEquals(dataSetRow["Column1"], DBNull.Value)))
        {
            MyModel.Column1 = Convert.ToString(dataSetRow["Column1"]);
        }
    }
    if (theData.Columns.Contains("Column2"))
    {
        if ((!object.ReferenceEquals(dataSetRow["Column2"], DBNull.Value)))
        {
            MyModel.Column2 = Convert.ToString(dataSetRow["Column2"]);
        }
    }
    if (theData.Columns.Contains("Column3"))
    {
        if ((!object.ReferenceEquals(dataSetRow["Column3"], DBNull.Value)))
        {
            MyModel.Column3 = Convert.ToString(dataSetRow["Column3"]);
        }
    }
    if (theData.Columns.Contains("Column4"))
    {
        if ((!object.ReferenceEquals(dataSetRow["Column4"], DBNull.Value)))
        {
            MyModel.Column4 = Convert.ToString(dataSetRow["Column4"]);
        }
    }

Solution

Useful property names:

I find using names as theData bad practice. It doesn't give any info on the instance. Give it a useful name you, and others, easily understand.

Casing of property names:

Don't use PascalCase for local fields, use camelCase instead. Your MyModel will become myModel. You had already done this correctly for dataSetRow.

Redundant parentheses:

Following:

if ((!object.ReferenceEquals(dataSetRow["Column1"], DBNull.Value)))


can be replaced by:

if (!object.ReferenceEquals(dataSetRow["Column1"], DBNull.Value))


There's no need to encapsulate the whole expression again in parantheses.

Simplifying the code:

Finally, the important part! :) There's only one way I can think of to achieve this: reflection. I created following extension method to get the PropertyInfo of your instance (note that it doesn't have to be placed in an extension method, I only think it looks cleaner and is good for reusability elsewhere if needed):

public static class Extensions
{
    public static PropertyInfo GetProperty(this T obj, string propertyName) where T : class
    {
        return typeof(T).GetProperty(propertyName);
    }
}


The above allows you to do following (get a property by it's name-value):

public class User
{
    public int ID { get; set; }
    public string Name { get; set; }
}

var user = new User { ID = 1, Name = "Simsons" };
var nameProp = user.GetProperty("Name");


And now you can get and/or set the value of that property using the GetValue() or SetValue() methods:

nameProp.SetValue(user, "Abbas", null);


With this in mind, add following changes to your code:

  • place all your column names in an array over which you will loop



  • simplify your code using the reflection part



Here's your code with the above applied:

theData = GetData();

if (theData.Rows.Count > 0)
{
    myModel = new CustomModel();
    dataSetRow = theData.Rows[0];
    var columnNames = new [] { "Column1", "Column2", "Column3", "Column4" };

    foreach(var columName in columnNames)
    {
        if(theData.Columns.Contains(columName))
        {
            if (!object.ReferenceEquals(dataSetRow[columName], DBNull.Value))
            {
                var columnProperty = myModel.GetProperty(columName);
                columnProperty.SetValue(myModel, Convert.ToString(dataSetRow[columnName], null);
            }
        }
    }
}


Update:

As RobH stated in the comments, you can use LinQ to avoid nested if statements. Here's how your code would look like:

var columnNames = new [] { "Column1", "Column2", "Column3", "Column4" };

foreach(var existingColumn in columnNames.Where(x => theData.Columns.Contains(x)))
{
    if (!object.ReferenceEquals(dataSetRow[existingColumn], DBNull.Value))
    {
        var columnProperty = myModel.GetProperty(existingColumn);
        columnProperty.SetValue(myModel, Convert.ToString(dataSetRow[existingColumn], null);
    }
}


You can even take it a final step further to avoid any if statement:

var validColumns = columnNames.Where(x => theData.Columns.Contains(x) &&
                                          !object.ReferenceEquals(dataSetRow[x], DBNull.Value)))

foreach(var column in validColumns)
{
    var columnProperty = myModel.GetProperty(existingColumn);
    columnProperty.SetValue(myModel, Convert.ToString(dataSetRow[existingColumn], null);
}

Code Snippets

if ((!object.ReferenceEquals(dataSetRow["Column1"], DBNull.Value)))
if (!object.ReferenceEquals(dataSetRow["Column1"], DBNull.Value))
public static class Extensions
{
    public static PropertyInfo GetProperty<T>(this T obj, string propertyName) where T : class
    {
        return typeof(T).GetProperty(propertyName);
    }
}
public class User
{
    public int ID { get; set; }
    public string Name { get; set; }
}

var user = new User { ID = 1, Name = "Simsons" };
var nameProp = user.GetProperty("Name");
nameProp.SetValue(user, "Abbas", null);

Context

StackExchange Code Review Q#78863, answer score: 4

Revisions (0)

No revisions yet.