Most Efficient Way To Get A Row Of Data From DB In ASP.NET

Question

Hi guys and girls,

I'm writing a method to return an 'asset' row from the database. It contains strings, ints and a byte array (this could be an image/movie/document).

Now for most row access I use the following method which returns a NameValueCollection as it is a light weight object, easy to use and cast int and strings.

        public static NameValueCollection ReturnNameValueCollection(Database db, DbCommand dbCommand)
    {

        var nvc = new NameValueCollection();

        using (IDataReader dr = db.ExecuteReader(dbCommand))
        {
            if (dr != null)
            {
                 while (dr.Read())
                 {
                     for (int count = 0; count < dr.FieldCount; count++)
                     {
                         nvc[dr.GetName(count)] = dr.GetValue(count).ToString();
                     }
                 }
            }
        }

        dbCommand.Dispose();
        return nvc.Count != 0 ? nvc : null;
    }

Now my apporach for this kind of data access would normally be to get a method to return a datarow.

       public static DataRow ReturnDataRow(Database db, DbCommand dbCommand)
    {
        var dt = new DataTable();

        using (IDataReader dr = db.ExecuteReader(dbCommand))
            if (dr != null) dt.Load(dr);

        dbCommand.Dispose();
        return dt.Rows.Count != 0 ? dt.Rows[0] : null;
    }

It does seem kind of wastefull to create a DataTable and then return its first datarow.

Is there better way to do this?

I'm thinking maybe a Dictionary of objects which I then manually cast each member of.

Would be interesting to see how others have tackled this. I know this kinda falls into the field of micro optimisation and as long as I'm not returning DataSets for each row query (wish I had a pound for everytime I saw that in a line of code) it should be ok.

That said this method is likely to be called for allot of data access queries on allot of sites on one box.

Cheers

Steve

Answer 1

What you're demonstarting is a code smell called Primitive Obsession. Create a custom type and return that from your repository method. Don't try to be overly generic... you'll just end up pushing that complexity into your business code because you'll be interacting with your entities using purely procedural code. Better to create objects that model your business.

If you're concerned with too much data access code, look in to using an ORM framework to take care of generating this for you. You shouldn't let this concern dictate bad design in your application layer.

Answer 2

In terms of efficiency of code, you've probably done it in the least keystrokes, and while it seems wasteful, is probably the simplest to maintain. However, if you're all about efficiency of only doing what is strictly necessary you can create a lightweight struct/class to populate with the data and use something similar to:

public class MyAsset
{
    public int ID;
    public string Name;
    public string Description;
}

public MyAsset GetAsset(IDBConnection con, Int AssetId)
{
    using (var cmd = con.CreateCommand("sp_GetAsset"))
    {
        cmd.CommandType = CommandType.StoredProcedure;
        cmd.Parameters.Add(cmd.CreateParameter("AssetID"));
        using(IDataReader dr = cmd.ExecuteReader())
        {
            if (!dr.Read()) return null;

            return new MyAsset() { 
                ID = dr.GetInt32(0), 
                Name = dr.GetString(1), 
                Description = dr.GetString(2)
            };
        }
    }
}

Likewise, you could dump the data in similar fashion right into your collection of KVPs...

It's not quite as clean looking as your original code, but it doesn't create the whole table just to get the single row...

As has been mentioned in another post regarding the code smell though, I probably wouldn't pass the command in as a parameter, I think I would be more likely to encapsulate the command inside this method, passing only the database connection and the id of the asset I wanted - assuming I didn't use caching of course, and passing back out the MyAsset instance. This keeps the method generic enough that it could be used on any database type - assuming the stored proc existed of course. This way the rest of my code is shielded from needing to know anything about the database other than what type of database it is... and throughout the rest of my app, I can reference asset info using MyAssetInstance.ID, MyAssetInstance.Name, MyAssetInstance.Description etc...

Answer 3

You will get much more benefit from caching data than trying to optimize returning a single row. If you're selecting by primary key then it's unlikely that you'll see any difference between returning a DataTable or a DataRow or a custom object. This strikes me as premature optimization. I would be more definite but I'm not sure if having a byte array in the mix changes things.

Answer 4

Hi Steve, how's it going?

Is there a reason you don't have object containers that represent a row in your database? Creating a custom object is much easier to deal with in other tiers of your solution. So, going with this approach, there are two very viable solutions to your problems.

Say you have a custom object that represents a Product in your database. You'd define the object like this:

public class Product {
    public int ProductID { get; set; }
    public string Name { get; set; }
    public byte[] Image { get; set; }
}

And you'd fill it like this:

using (var reader = command.ExecuteReader()) {
    if (reader.Read()) {
        var product = new Product();

        int ordinal = reader.GetOrdinal("ProductID");
        if (!reader.IsDBNull(ordinal) {
            product.ProductID = reader.GetInt32(ordinal);
        }

        ordinal = reader.GetOrdinal("Name");
        if (!reader.IsDBNull(ordinal)) {
            product.Name = reader.GetString(ordinal);
        }

        ordinal = reader.GetOrdinal("Image");
        if (!reader.IsDBNull(ordinal)) {
            var sqlBytes = reader.GetSqlBytes(ordinal);
            product.Image = sqlBytes.Value;
        }

        return product;
    }
}

Notice that I'm retrieving a value via the reader's Getx where x is the type that I want to retrieve from the column. This is Microsoft's recommended way of retrieving data for a column per http://msdn.microsoft.com/en-us/library/haa3afyz.aspx (second paragraph) because the retrieved value doesn't have to be boxed into System.Object and unboxed into a primitive type.

Since you mentioned that this method will be called many, many times, in an ASP.NET application, you may want to reconsider such a generic approach as this. The method you use to return a NameValueCollection is very non-performant in this scenario (and arguably in many other scenarios). Not to mention that you convert each database column to a string without accounting for the current user's Culture, and Culture is an important consideration in an ASP.NET application. I'd argue that this NameValueCollection shouldn't be used in your other development efforts as well. I could go on and on about this, but I'll save you my rants.

Of course, if you're going to be creating objects that directly map to your tables, you might as well look into LINQ to SQL or the ADO.NET Entity Framework. You'll be happy you did.

Answer 5

Thanks for all the input guys. I know ORM is probably the way to go and that and the MVC framework are next on my list.

To give a bit more detail, the code I'm showing is from the helpers section in my data access layer which then passes the row or name value collection to the business layer to turn into objects.

I think mnero0429 and balabaster code examples give me the right direction. Use a datareader and manually get the data out like that without messing around with intemediary objects. Thanks for the detailed MS link mnero0429. Fair on on the primative obsession - tho I really do make a proper asset class out of it in the business layer ;)

I'll be looking into the ADO entity framework too.

Once again, thanks for the advice - I know the world would keep turning even if I used DataSet.Tables[0].Rows[0]["bob"] or some such but when you get that itch - whats the BEST wat to do it, its nice to have it scratched!