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

Converting List to a DataTable and/or DataSet Extension Methods

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

Problem

Can someone help me improve this code? I and trying to have a couple extension methods to convert strongly-typed lists to a DataSet and DataTable respectively.

But... being so green to C#, I am not sure this is the most efficient way to do this.

```
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Reflection;

namespace o7th.Class.Library.Data
{
///
/// List to DataTable Converter
///
public static class ListConverter
{

///
/// Convert our List to a DataTable
///
///
///
/// DataTable
public static DataTable ToDataTable(this IList data)
{
PropertyDescriptorCollection props = TypeDescriptor.GetProperties(typeof(T));
object[] values = new object[props.Count];
using (DataTable table = new DataTable())
{
long _pCt = props.Count;
for (int i = 0; i
/// Convert our List to a DataSet
///
///
///
/// DataSet
public static DataSet ToDataSet(this IList list)
{
Type elementType = typeof(T);
using (DataSet ds = new DataSet())
{
using (DataTable t = new DataTable())
{
ds.Tables.Add(t);
//add a column to table for each public property on T
PropertyInfo[] _props = elementType.GetProperties();
foreach (PropertyInfo propInfo in _props)
{
Type _pi = propInfo.PropertyType;
Type ColType = Nullable.GetUnderlyingType(_pi) ?? _pi;
t.Columns.Add(propInfo.Name, ColType);
}
//go through each property on T and add each value to the table
foreach (T item in list)
{
DataRow row = t.NewRow();
foreach (PropertyInfo propInfo in _props)
{
row[propInfo.Name] = propInfo.GetValue(it

Solution

There are a few things I notice right off the bat.

-
Your variable names while not bad, could be better. For instance props -> properties. Stuff like this makes the code easier to read.

-
You have the properties, why not use a foreach loop to fill the datatable (you did it in ToDataSet)

-
the _ prefex should be used for class variables, not local variables.

-
try using var when declaring obvious variable types var row = t.NewRow()

-
There is no error checking when you are filling the values in the data table. What happens if it is not a class (int, double, long)? You could force the generic to be a class by adding where T : class.

-
Why don't you use the ToDataTable method to create the table in the ToDataSet method? This will eliminate duplicate code, and have 1 point of failure/modification as required. As an aside, I would use the code from ToDataSet to create your DataTable, as it is written better.

-
While I applaud your use of it, I'm not sure the using syntax is appropriate here. I would move that to where these methods are being called using (var dt = list.ToDataTable()) Having it here will more than likely cause unexpected things to happen in your code.

-
I would make these extend IEnumerable as that will make them way more useful by not limiting them to IList.

I do like your use of white space and indentation, so good job on that. The extra indentation will be removed when the using statements are removed. I also like the name of your methods, very clear and concise to their intent.

Context

StackExchange Code Review Q#40891, answer score: 8

Revisions (0)

No revisions yet.