Naor
Naor

Reputation: 24063

fetch datarow to c# object

I have an class Item that represents an item in a list. I have in it function that calls stored procedure that returns datatable and I need to convert the datatable to Array of items. Here is what I do:

public class Item
{
    private string _ItemIdDataName = "item_id";
    private string _ItemNameDataName = "item_name";
    private string _PriceDataName = "price";

    public long ItemId { get; set; }
    public string ItemName { get; set; }
    public float Price { get; set; }

    private Item(DataRow row)
    {
        if (row != null)
        {
            ItemId = long.Parse(row[_ItemIdDataName].ToString());
            ItemName = row[_ItemNameDataName].ToString();
            Price = float.Parse(row[_PriceDataName].ToString());
        }
    }

    public Item[] load()
    {
        DataTable dt=DBHandler.GetItems();//Stored procedure that returns DataTable 
        Item[] items = new Item[dt.Rows.Count];
        for (int i = 0; i < dt.Rows.Count; i++)
        {
            items[i] = new Item(dt.Rows[i]);
        }
        return items;
    }
}

Am I doing it right? How can I improve this?

Upvotes: 7

Views: 20533

Answers (4)

Tomas Jansson
Tomas Jansson

Reputation: 23462

If you're only gonna use it once it probably fine, but if you'll do it a lot you should try to do some more generic stuff. I wrote a blog post about how to write an extension method for DataTable that creates a list of objects. It works by the convention that the properties in the object should have the same name as the columns in the stored procedure (I would change the name in the stored procedure if I could):

public static class DataTableExtensions
{
    public static IList<T> ToList<T>(this DataTable table) where T : new()
    {
        IList<PropertyInfo> properties = typeof(T).GetProperties().ToList();
        IList<T> result = new List<T>();

        foreach (var row in table.Rows)
        {
            var item = CreateItemFromRow<T>((DataRow)row, properties);
            result.Add(item);
        }

        return result;
    }

    public static IList<T> ToList<T>(this DataTable table, Dictionary<string, string> mappings) where T : new()
    {
        IList<PropertyInfo> properties = typeof(T).GetProperties().ToList();
        IList<T> result = new List<T>();

        foreach (var row in table.Rows)
        {
            var item = CreateItemFromRow<T>((DataRow)row, properties, mappings);
            result.Add(item);
        }

        return result;
    }

    private static T CreateItemFromRow<T>(DataRow row, IList<PropertyInfo> properties) where T : new()
    {
        T item = new T();
        foreach (var property in properties)
        {
            property.SetValue(item, row[property.Name], null);
        }
        return item;
    }

    private static T CreateItemFromRow<T>(DataRow row, IList<PropertyInfo> properties, Dictionary<string, string> mappings) where T : new()
    {
        T item = new T();
        foreach (var property in properties)
        {
            if(mappings.ContainsKey(property.Name))
                property.SetValue(item, row[mappings[property.Name]], null);
        }
        return item;
    }
}

Now you can just call

var items = dt.ToList<Item>();

or

var mappings = new Dictionary<string,string>();
mappings.Add("ItemId", "item_id");
mappings.Add("ItemName ", "item_name");
mappings.Add("Price ", "price);
var items = dt.ToList<Item>(mappings);

The blog post is here: http://blog.tomasjansson.com/2010/11/convert-datatable-to-generic-list-extension

There are many ways in which you can extend this, you could include some kind of mapping dictionary telling the extension how to map the columns, in that way the names doesn't need to match. Or you can add a list of property names that you would like to exclude in the mapping.

Update: Your object (Item) you are creating must have a default constructor otherwise the private method won't be able to create it. Since the way the solution works is first to create the object than use the properties that you get from the reflection to set the values of the object.

Update 2: I added the part with mappings dictionary but haven't tried it myself so it might not compile. However, the concept is there and I think it works.

Upvotes: 26

dance2die
dance2die

Reputation: 36905

Your private Item constructor and the load() functions do not seem to belong in your Item class.

You know, a class should do one thing and one thing well.

So try to
1. refactor out the private c'tor to a say a helper class that simply parses the DataRow and returns an instance of Item
2. and refactor the load() into a different class that simply uses the above helper method and returns an array of Item object instances

Upvotes: 0

user111013
user111013

Reputation:

Pretty good, but I have a few suggestions:

  • Don't cast things ToString to be parsed back to another type. This can cause corruption of your data type, and is slow/inefficient.

  • Expect and check for null's coming from SQL Server.

So, instead of:

ItemId = long.Parse(row[_ItemIdDataName].ToString());

Try:

 ItemId = row.Field<long?>(_ItemIdDataName) ?? value_if_null; 

(Add a reference to System.Data.DatasetExtensions to get the Field extension)

Upvotes: 5

Related Questions