patterncsharpMinor
Getting column values from a datatable
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
Casing of property names:
Don't use PascalCase for local fields, use camelCase instead. Your
Redundant parentheses:
Following:
can be replaced by:
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
The above allows you to do following (get a property by it's name-value):
And now you can get and/or set the value of that property using the GetValue() or SetValue() methods:
With this in mind, add following changes to your code:
Here's your code with the above applied:
Update:
As RobH stated in the comments, you can use LinQ to avoid nested if statements. Here's how your code would look like:
You can even take it a final step further to avoid any if statement:
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.