SQL Connections (C#) best practice?

[benpaul]

Hi All,

 

Brand new to the forum, I have been reading for a short while and have found lots of intresting threads that have helped me begin with .NET and specifically (C#) so thanks for that!

 

Anyways, here is what I have been up to while learning... I am writing a windows forms application that utilizes an SQL SERVER EXPRESS database to store and retreive data.

 

What I would like your views on really, are the best practices when retreiving data from the database... at the moment I have a class which basically contains a set of methods that I use to retreive different bits of data from my database... for example...

 

private string connectionString = ConfigurationManager.ConnectionStrings["MyConnectionString"].ConnectionString;

public decimal getReceiptsTotal(int eventID)
{
  string source = connectionString;
  string select = "SELECT amount FROM receipts WHERE eventid="+ eventID;

  SqlConnection conn = new SqlConnection(source);

  conn.Open();
  SqlCommand cmd = new SqlCommand(select, conn);
  SqlDataReader reader = cmd.ExecuteReader();

  decimal total = 0.00m;

  while (reader.Read())
  {
      total = total + (decimal)reader[0];
  }
  conn.Close();
  return total;

 

In the above code, I have a table called 'Receipts' which contains an eventid and an amount... what it does is grab all of the receipts for that event in the database and adds them together, finally returning the result.

(Im sure its not amazingly pretty code but it works).

 

Now.... this class holds lots of similar methods that return various bits of data from the database, not all of them decimals... I have a function that returns the event name for example, as a string.

 

Obviously, each time a method is called... it creates a new connection to the database, sends the SELECT statement, grabs the result and returns it... is this good practice? Creating a connection to the database in the above way everytime I want some data from the database? it seems logical to me to do it this way, mainly becuase it works I guess... but just wanted to get some views on how others would do the same transactions.

 

 

This line...

 

private string connectionString = ConfigurationManager.ConnectionStrings["MyConnectionString"].ConnectionString;

 

This is grabbing my connection string from the app Configuration, and is accessable by the whole class so each method just calls the variable 'connectionString' when it needs to create the connection, I dont declare this more than once in the whole app.

 

 

 

I hope this is clear to you all, and I look forward to your replys!

 

 

Thanks in advance!

 

 

Ben

[PlausiblyDamp]

If I was doing all the work myself I would tend to put the code for creating connections, commands etc. into a class of it's own and call this class from the slightly higher level methods like getReceiptsTotal etc.

 

This class would have fairly simple methods like GetConnection() which would just return a valid connection object and slightly more complex methods such as GetCommand or GetReader which would return the corresponding object based on parameters passed in. Typically I would overload these to cover the most common scenarios.

 

http://www.microsoft.com/downloads/details.aspx?familyid=f63d1f0a-9877-4a7b-88ec-0426b48df275&displaylang=en can be used as a model (or even as is) as it covers the most common scenarios.

 

Opening and closing a connection every time isn't that big an overhead due to connection pooling being used behind the scenes to reduce the actual connections being really created and destroyed - and very slight performance hit is far outweighed by the fact you aren't leaking connections. One thing I would tend towards though is the using statement as this will make sure connections are closed even when exceptions are thrown.

 

Other than that I would really avoid using string concatenation to build your queries, parameterised queries are far safer and can be converted to stored procedures if need be with very little effort. http://www.xtremedotnettalk.com/showthread.php?p=463520#post463520 has a bit more background on other reasons why concatenation is a bad thing...

[benpaul]

Fantastic, thankyou very much for your excellent help!

I kind of grasped that it would be best to have a higher level class that dealt with things like creating a connection etc, if nothing else to stop replication of code each time I want to retreive data, your links have provided a good source for me to read from in order to get closer to that.

 

Many Thanks!

[Diesel]

I would also recommend using the factory pattern in ADO.Net instead of constructors.

 

This way, you could switch the type of storage with a singe line of code or configuration setting.

 

http://msdn.microsoft.com/en-us/library/ms971499.aspx

[SNK111]

Check this article HOW TO: SQL & C# for details on how to connect to SQL Server database from C#.NET database applications as well as Java database applications. It also describes how to pass embedded SQL queries, calling stored procedures, pass parameters etc.