views:

265

answers:

5

We have a SQL utility class that takes the name of a stored procedure an its input parameters, and returns the results in datatable. The reasoning behind this is so that we don't have to worry about forgetting to close connections and having connection leaks. Also so that we can reduce code by not having to recreate datadapters and datareaders in our data access layers.

The problem I have with this is that we're populating a datatable so that we can loop through it to create our objects, so we're basically using it like a datareader. I've read about classes that will return a datareader or dataadapter. But the problem with this is either client has to open and close connections, or you have to close the connection in a Finalize method. It seems that you wouldn't want garbage collection being responsible for closing your database connections.

To sum up, we want to have a class so that we can reduce code by not having to create datareaders for every query and so that we can ensure database connections are closed.

What is the best way of handling this?

UPDATE: Still thinking about this, but so far it seems that the best practice is to still return a datareader, use CommandBehavior.CloseConnection, and then trust who ever uses the class to call dr.Close()?

+1  A: 

I have the same structure - utility classes with methods that fetch the data and return filled DataTables (or fill/update a DataTable passed in to them) - for exactly the same reasons: keeping the database connections separate from the rest of the code and ensuring they are opened only when required and closed asap. Especially since the data is stored in various back-end systems, and I want to present only one interface to my application and not have it worry about the details.

There is one difference to your situation: We don't (in general) create objects from the rows in the DataTables, but rather work directly on the data in the rows. I find working with DataTables simple and efficient.

Other than that, I personally don't see anything wrong with this approach and find that it works very well for our purposes.

Tom Juergens
+1  A: 

Have you considered the Microsoft Enterprise Library?

public List<User> GetUsers()
{
    List<User> result = new List<User>();

    Database db = new
    Microsoft.Practices.EnterpriseLibrary.Data.Sql.SqlDatabase(this.connectionString);
    DbCommand cmd = db.GetStoredProcCommand("GetUsers");

    using (IDataReader rdr = db.ExecuteReader(cmd))
    {
        while (rdr.Read())
        {
            User user = new User();
            FillUser(rdr, user);                   
            result.Add(user);
        }
    }
    return result;

}
rick schott
Thanks for this suggestion. This looks interesting.
Chad
+2  A: 

We use something like this and it performs very well under high volume.

    public SqlDataReader ExecuteReader(string command, SqlParameter[] parameters)
    {
        SqlDataReader reader = null;

        using (SqlConnection conn = new SqlConnection())
        using (SqlCommand cmd = conn.CreateCommand())
        {
            conn.Open();

            cmd.CommandText = command;
            cmd.CommandType = CommandType.StoredProcedure;

            cmd.Parameters.AddRange(parameters);

            reader = cmd.ExecuteReader(CommandBehavior.CloseConnection);
        }

        return reader;
    }

DataTables are not considered best practice for several reasons including their bloat and lack of type safety.

Jason
Also, if you're interested in filling objects from a data reader, consider using something like http://www.codeproject.com/KB/database/NullableReaders.aspx
Jason
One of the things we wanted to address was not having to worry about closing connections. With this you still have to manually call reader.Close to close the connection, correct?
Chad
@Chad Close will occur at the end of the using statement automatically, so you don't need to actually add it in the code. Note that this doesn't necessarily close the user from the thread pool that is occurring on the server, so if you keep sending commands, you'll still get the time benefit from the thread pool on the server. I guess I should also mention that it's a bad idea to not close your connection.
Jage
@Jage: the goal was to encapsulate it so someone doesn't forget. Someone wrote a class that returned a datatable even when we didnt need it. I hated that solution so I was looking for an alternative. Since I wrote this, I never really found a solution I liked, so I've just been doing it in my data access code manually.
Chad
A: 

Returning a datareader doesn't work in a lot of scenarios. At a lot of places, direct connections to the database from the client machine are not allowed in production (for good reason). So you have to serialize the objects you are retrieving. I can think of designs that would allow you to persist a datareader in whatever class you use for remoting/serialization on the server side but returning items across http or nettcp in row by agonizing row fashion likely does not offer much benefit.

Are you serializing these objects? If so, your choices boil down to Datatable, Dataset, or custom objects. Custom objects, if written well, perform and serialize the best but you have to write concurrency in addition to a bunch of other functionality.

IMO, since ADO.Net 2.0, datatables can perform well even in large scale remoting situations. They provide a special binary remoting format and are simple to work with. Throw in some compression and you're not even using a lot of bandwidth for your large data sets.

marr75
The basic idea is just to return a datareader so we can loop through the query results and fill our objects. I'm just not sure if that is the best way to go about it.My beef is that right now we're returning a datatable so that we can loop through the rows to create our objects(essentially using it as a datareader). Doesn't seem right to me.
Chad
A: 

well, if you plan to use this class inside of web pages you can register the utility class with the page's unload event. In the event sink you can write your logic to close the database connection. Check out this tip on codeproject for more ideas.

however this solution won't work for use inside web methods (web services). I suppose you'd have to adapt the technique for web service use. Your last line in the web method should should be an event call. So when you write your web service class, define an event called WebMethodCompleted. You'd probably get a reference to the instance of the web service via the technique mentioned in the article. Once you get a reference you can register tthe event in your utility class. Just remember to invoke the event in the web method.

Happy programming.

deostroll