0

I was thinking about building a method that I could pass in the connString to in order to generate my SqlConnection.

Is that a bad idea? Is it better if I just wrap my calls in their own using (SqlConnection statements?

We had issues with another app where one of the devs was passing the connection around and keeping it open, which was causing a lot of grief.

Just wanted to ask the experts before attempting something that could lead to issues.

Oded
  • 489,969
  • 99
  • 883
  • 1,009
scarpacci
  • 8,957
  • 16
  • 79
  • 144
  • 2
    Show an example of how you're planning to use it. – Yuck Dec 12 '11 at 14:13
  • That's a perfectly good idea compared to the existing functionality. The question is: is it enough? I'm with @Yuck: let's see how you want to use it. – Cᴏʀʏ Dec 12 '11 at 14:14
  • make a DataAccess class library which exposes entities or methods like GetTable, ExecuteCommand etc but never expose any detail of such class library to the calling Business Logic or UI (if you are lazy and do not want to create a BL class library). In the DataAccess classes, use using around every single connection as you already described, when your retrieve a DataReader you cannot use the using around the connection but you can create the reader telling to close connection when reader is closed, in the calling code put a using around the reader. – Davide Piras Dec 12 '11 at 14:16
  • 1
    @DavidePiras So, in essence - use Entity Framework? :P – Yuck Dec 12 '11 at 14:17
  • Why was it causing a lot of grief? Other devlopers accidentally closing the connection, or spawning too many connections, hogging resources? –  Dec 12 '11 at 14:18
  • No, I did not mean so, but if you do, have a look at my answer here with layered approach suggested: http://stackoverflow.com/a/7474357/559144 – Davide Piras Dec 12 '11 at 14:18
  • @AndersUP it was spawning too many connections – scarpacci Dec 12 '11 at 14:35
  • @scarpacci Then certainly go with the using construct. –  Dec 12 '11 at 15:04

3 Answers3

0

This is the common pattern I use and I think it's good:

using (var cn = new SqlConnection(DatabaseConnection.ConnectionStringToDb))
{
    using (var cmd = new SqlCommand("command string", cn))
    {
          // your code
    }
}

Where DatabaseConnection.ConnectionStringToDb is a static helper class property I create to fetch or store connection string.

Yuck
  • 49,664
  • 13
  • 105
  • 135
Shekhar_Pro
  • 18,056
  • 9
  • 55
  • 79
  • 1
    I use kind of the same, but you can do cn.CreateCommand("select...") it's nicer – Davide Piras Dec 12 '11 at 14:23
  • 1
    yeah.. though you dont have a [CreateCommand()](http://msdn.microsoft.com/en-us/library/1756xwa3.aspx) which takes a parameter.. i guess it was a typo.. – Shekhar_Pro Dec 12 '11 at 14:30
0

I was thinking about building a method that I could pass in the connString to in order to generate my SqlConnection. Is that a bad idea?

It's a great idea. But Microsoft beat you to it with this constructor on SqlConnection:

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

Ian Nelson
  • 57,123
  • 20
  • 76
  • 103
  • using that example when does the connection get closed? ? – scarpacci Dec 12 '11 at 14:36
  • 1
    @scarpacci - under *your* suggested implementation, when does the connection get closed. All we can see of your suggestion is a method that, given a connection string, returns a connection object - which sounds pretty much like what `new SqlConnection` is doing... – Damien_The_Unbeliever Dec 12 '11 at 15:00
-1

Don't keep the connection open. If you use everywhere using(SqlConnection con = new SqlConnection(...)) .Net uses connection pooling that works very fast. You can read about that here http://msdn.microsoft.com/en-us/library/8xx3tyca.aspx

Eugene
  • 1,515
  • 1
  • 13
  • 23
  • I don't think the OP is arguing **in favor of** keeping connections open. In fact, he mentions that doing so had caused problems in another application. – Yuck Dec 12 '11 at 14:16