Getting column values from a datatable

Question

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"]);
 }
 }

• MrSmith42: I see 4 very similar blocks. Reduce them to one by using a loop over the constants "Column1", "Column2", "Column3", "Column4"
• Heslacher: Is this real code ?
• Abbas: @MrSmith42 MyModel.Column3 would not be replacable by MyModel["Column3"] or something similar as it is a property of a class.
• MrSmith42: @Abbas You are right but you can still use a loop and use the loop index to select 'MyModel.Column1' to 'MyModel.Column4' which you stored in an array.
• Abbas: How are you going to select the property with an indexer without using reflection?

Answer 1

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<T>(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);
}

• RobH: +1. You could convert most of that foreach to LINQ though, might look a bit nicer than the nested ifs.
• Abbas: @RobH Thanks for the tip, updated my answer. ;)

Answer 2

If you can refactor your model class so that the column names and values are in a dictionary instead of separate properties everything an be simplified to one LINQ statement:

class CustomModel
{
 public Dictionary<string, string> ColumnValues = new Dictionary<string, string>
 {
 { "Column1", "" },
 { "Column2", "" },
 { "Column3", "" },
 { "Column4", "" }
 };
}
var dataSetRow = theData.Rows[0];
CustomModel MyModel = new CustomModel();
MyModel.ColumnValues = (from DataColumn c in theData.Columns
 where MyModel.ColumnValues.ContainsKey(c.ColumnName) && dataSetRow[c] != DBNull.Value
 select new KeyValuePair<string, string>(c.ColumnName, dataSetRow[c].ToString())).ToDictionary(x => x.Key, x => x.Value);