How to manage SQL connections

I have an ASP.NET/C# application (.NET 2.0) that is basically a data entry interface to a SQL database. I'm trying to figure out the best way to manage SQL connections within it.

What I'm doing now, which isn't working, is I have 2 protected classes:

The first takes a command, opens a connection, and returns a datareader. The problem is, for the datareader
to work, I need to leave the connection open. Once this function exits, I have no way to close the connection.


 protected SqlDataReader sql_read(string lc_cmd)
    {

        SqlConnection cn = new SqlConnection(System.Configuration.ConfigurationManager.AppSettings["ConnectionString"]);
        cn.Open();
        SqlCommand cmd = new SqlCommand(lc_cmd, cn);
        SqlDataReader dr = cmd.ExecuteReader();
        return dr;


    }


This second takes a command, opens a connection, and returns a datatable. This one is able to close the connection.

protected DataTable sql_run(string lc_cmd)
    {

        SqlConnection cn = new SqlConnection(System.Configuration.ConfigurationManager.AppSettings["ConnectionString"]);
        SqlDataAdapter cmd = new SqlDataAdapter(lc_cmd, cn);
        cmd.SelectCommand.CommandType = CommandType.Text;
        DataTable myDataTable = new DataTable();
        cmd.Fill(myDataTable);
        cmd.Dispose();
        cn.Close();

        return myDataTable;
    }


What I want is one simple global class that opens the sql connection as part of the constructor, allows fluid access of the connection, and then closes it on the deconstructor. I tried to add in this code:

 SqlConnection myConn = new SqlConnection(System.Configuration.ConfigurationManager.AppSettings["ConnectionString"]);

    public batch2()
    {      
        //SqlConnection myConn = new SqlConnection(System.Configuration.ConfigurationManager.AppSettings["ConnectionString"]);
        myConn.Open();
    }

    ~batch2()
    {
        myConn.Close();
    }

and reference myConn inside the class, but it just crashes after I access myConn twice.

Any advice appreciated! There is a lot of SQL read/write so I think I'll be better served leaving the connection open, although if I had just one class I could set a timer object to close the connection if no activity came within the past 30 seconds, and reopen it when needed.
If it makes a difference, I have several pages (I guess, classes), so something that could be global to the entire project would be ideal.


Thanks,
Terry


TerryBurgerAsked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

GavinMannionCommented:
This is what I have and I don't seem to run into any problems ;)

public DataSet ACMBFill(SqlCommand SQLCmd, string tableName)
            {            
                  bool ManageConnection = false;
                  DataSet ReturnSet = new DataSet();
                  
                  if( SQLCmd != null)
                  {
                        try
                        {
                              if (SQLCmd.Connection.State != ConnectionState.Open)
                              {
                                    SQLCmd.Connection.Open();
                                    ManageConnection = true;
                              }
                              SqlDataAdapter da = new SqlDataAdapter(SQLCmd);
                              da.Fill(ReturnSet, tableName);
                              da = null;
                        }
                        finally
                        {
                              if (ManageConnection)
                                    SQLCmd.Connection.Close();
                        }
                        return ReturnSet;
                  }
                  return null;
            }

Basically the Finally statement is important because it will run after the try and catch section no matter what happens which makes sure the connection gets closed.

I have a bool value to check whether or not my connection is open already and if it is then just use that one.
I send the SqlCommand object to the function, that is set up as required in my business layer.

Let me know if you need some more help...
0
junglerover77Commented:
You are absolutely wrong to think it's necessary to leave the connection open after "ExecuteReader" for the datereader to work. Just try the following:

 protected SqlDataReader sql_read(string lc_cmd)
    {

        SqlConnection cn = new SqlConnection(System.Configuration.ConfigurationManager.AppSettings["ConnectionString"]);
        cn.Open();
        SqlCommand cmd = new SqlCommand(lc_cmd, cn);
        SqlDataReader dr = cmd.ExecuteReader();

        cn.Close();  
        return dr;
    }

It still works.
0
AGBrownCommented:
(See end of post for comment on junglerover77's last post.)

Terry,

Best practice for using SqlClient objects is open to personal preference. I would stear clear of trying to open a connection in the constructor, and close it in the destructor - that method is liable to leave connections open for a long time, which will severely affect your application performance. There are other options though, which are just as good.

Gavin's code is a good example of a methodology. Note that you don't need to test for a connection's state before opening it if you are only opening it, however in Gavin's code it is necessary in order to set the ManageConnection flag. You could just as easily substitute:
    ManageConnection = (SQLCmd.Connection.State != ConnectionState.Open);
    SQLCmd.Connection.Open();

For an example of a plugnplay code block which encapsulates the common parts of calling SqlClient objects take a look at the DAB 2.0 (http://www.microsoft.com/downloads/details.aspx?FamilyId=F63D1F0A-9877-4A7B-88EC-0426B48DF275&displaylang=en). I point to this one instead of the latest Enterprise Library version as its a lot simpler to understand and use. It shows a reasonable (not optimal) method of taking most of the effort out of writing your SqlClient code. It is very fast to drop in and use, though it doesn't prevent all the common code.

The difficulty that you have addressed with the Reader object is something that you can't effectively get around unless you start to use abstract classes with abstract methods, or delegates/events. I'll illustrate the event model:

public class ReaderManager()
{
      public void ExecuteReader()
      {
            SqlDataReader reader;
            SqlConnection conn = new SqlConnection("");
            SqlCommand cmd = conn.CreateCommand();
            cmd.CommandText = "";
            cmd.CommandType = CommandType.StoredProcedure;

            if (this.BeforeReaderFilled != null)
                  this.BeforeReaderFilled(this, new EventArgs());

            conn.Open();
            try
            {
                  reader = cmd.ExecuteReader();

                  if (this.ReaderFilled != null)
                        this.ReaderFilled(this, new EventArgs());

                  reader.Close();
                  if (this.ReaderClosed != null)
                        this.ReaderClosed(this, new EventArgs());
            }
            finally
            {
                  if (reader != null)
                        reader.Close();
                  conn.Close();
            }
      }
      public event EventHandler BeforeReaderFilled;
      public event EventHandler ReaderFilled;
      public event EventHandler ReaderClosed;
}

I've left this model very simple to illustrate what it does (and that includes not using particular good practice for the event calls; BeforeReader should really be raised using a private OnBeforeReader event to make the code more readable). In the BeforeReaderFilled event handler, your calling class would access the SqlCommand that you are using to add any necessary parameters to the command object. In the ReaderFilled event handler, your calling class would read through the reader, and in the ReaderClosed event handler, your calling class would access any output parameters. You can see that you could also pass a pre-prepared command object and a closed connection to a similar method to get the same end-effect.

In a practical implementation there is a bit you have to add according to how you want to use the class, such as setting the connection string and command text. You have two options to expose the command object, parameters for the command, and the reader itself to the calling class event handlers. The first is by simply making them class level fields (variables) and exposing them through properties (or just making them public and not using properties - but that is "bad practice" apparently). However the best option is to pass the command and the reader in the event arguments. This would require deriving an event argument class, and using a custom event handler such as:
      //      this code sits at namespace level
      public delegate void BeforeReaderFilledHandler(object sender, BeforeReaderFilledEventArgs e);
      public class BeforeReaderFilledEventArgs : EventArgs
      {
            public SqlCommand Command;
            public BeforeReaderFilledEventArgs(SqlCommand command)
            {
                  this.Command = command;
            }
      }
      
This changes your event declaration and call to:

      ...
      if (this.BeforeReaderFilled != null)
            this.BeforeReaderFilled(this, new BeforeReaderFilledEventArgs(cmd));
      ...
      public event BeforeReaderFilledHandler BeforeReaderFilled;
      
Again, I've left the other three out for brevity, but you would pass the reader and parameters in a similar manner.

The advantage of this approach is that you are guaranteed that you will always close the reader and connection after using the reader, and even if an exception is thrown in the calling class.

The abstract class approach would be to encapsulate this into an abstract class and instead of using events, call abstract methods which a concrete class will override to use the reader, and set/read the parameters. E.g.:

            ...
            //if (this.BeforeReaderFilled != null)
            //      this.BeforeReaderFilled(this, new EventArgs());
            this.ManipulateCommand(cmd);
            ...
      }
      abstract void ManipulateCommand(SqlCommand command);

Your code that uses a reader then inherits from that class and implements the UseReader.

W.r.t. open/close connections and readers:

junglerover77,

I don't agree with your last comment at all, though I may be missing your point. To use (read) a datareader your connection _must_ be open. If it is not then you will get an InvalidOperationException (Invalid attempt to Read when reader is closed.). From the MSDN documentation on the sqldatareader:

"While the SqlDataReader is being used, the associated SqlConnection is busy serving the SqlDataReader, and no other operations can be performed on the SqlConnection other than closing it. This is the case until the Close method of the SqlDataReader is called. For example, you cannot retrieve output parameters until after you call Close."

This means that you have to leave the connection open whilst reading the reader. Also, to use any output parameters SqlDataReader.Close() must have been called, but the connection does not have to be closed.

You can get the connection to close automatically when the reader is closed by using SqlCommand.ExecuteReader(CommandBehavior.CloseConnection). However, the reader must still be open for you to read it, and it is imperative that you close the reader or you will snarl up your connection pool very quickly.

Into your code put the following lines:
protected SqlDataReader sql_read(string lc_cmd)
    {

        SqlConnection cn = new SqlConnection(System.Configuration.ConfigurationManager.AppSettings["ConnectionString"]);
        cn.Open();
        SqlCommand cmd = new SqlCommand(lc_cmd, cn);
        SqlDataReader dr = cmd.ExecuteReader();

        cn.Close();  
        return dr;
    }

public void TestReader()
{
        SqlDataReader reader = sql_read("myCommand");

        while (reader.Read()) // this will throw an InvalidOperationException because of the cn.close inside sql_read
        {
                // production code does something here
        }
}
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
Cloud Class® Course: Amazon Web Services - Basic

Are you thinking about creating an Amazon Web Services account for your business? Not sure where to start? In this course you’ll get an overview of the history of AWS and take a tour of their user interface.

TerryBurgerAuthor Commented:
A lot of good info here. I'm still processing it, and will accept shortly. Thanks!


Terry
0
AGBrownCommented:
np
0
AGBrownCommented:
Terry,

There is more background to the reason why I ended up going to the event-based model for the datareader. Without the event-based model there is a very simple way of ensuring the datareader is closed properly and that is to use a using{} block: the datareader, sqlcommand and sqlconnection all implement IDisposable, so a using{} block ensures that they are always closed properly regardless of normal execution, or an exception state.

I use a similar model to the Enterprise Library to manage my database conversations where they do in fact recommend the using{} block. However, the Enterprise Library doesn't allow for a critical scenario in our application; where the database is taken offline either through an admin statement, changing the database state from read/write to read only, or through hardware restart. In this situation it is impractical and undesirable to restart the web farm. We therefore have a .NET 1.1 workaround, which to all extents is pretty much the same as the .NET 2.0 SqlConnection.ClearPool command. In this case, implementing using{} blocks becomes a nightmare. In addition, my database classes are actual instance-based objects (not static) each with a connection property, command property, transaction property etc. This means that those objects can span multiple interactions from a consuming object. As a result, the database connection is not opened until just before the command is executed and the using block is impractical.

Apart from anything else this model allows me to enforce the rule that our developers do not use the System.Data.SqlClient objects directly. Instead they go through a "managed" (not the same as the .NET definition of managed) data access layer. They are more than happy to do this as it reduces the amount of code that needs to be written considerably and the event-based model means that readers and connections are _always_ closed properly. Without enforcing this it is possible for a piece of code to slip through to production that doesn't close it's connection. If this happens then you will eventually come across a situation where (after that code has been hit many times consecutively) your customers will see failures in your database conversations.

I'm writing an article for my web site with code examples of this method, and integration with the enterprise library in .NET 1.1 and 2.0. I'll post links when it's done.

Andy
0
TerryBurgerAuthor Commented:
It makes pretty good sense to use a managed class.

If you have enough developers, is there a reason you haven't switched to a service based connection pool? I've been too busy
to do it here, but it sounds like you're running a much larger set of applications. Without doing the background I imagine you'd have a bit more overhead in the XML, but you'd also save on opening/closing connections. Maybe it would just as efficent in the end?



0
AGBrownCommented:
A connection pool service is a possibility - I take it you mean a remoting service? It isn't something I have yet looked at. I should imagine the problem with flushing the pool when the connections are invalidated would still be a problem.

The applications aren't particularly "large" in terms of usage. In one application the connections are used in the web application itself in a simple 2-tier architecture; there hasn't been any need to expand that application yet though it is layered to as to be able to do so when the need arises. Another application is more complex, with smart, rich and web clients and service-based architecture. This talks to the database in the bottom-most service layer. There is only one physical service implementation in this position at the moment and it uses a lot of datareaders to fill the data transfer objects.

Keeping smaller individual connection pools on each web service would intuitively seem to have a performance advantage over using a central remoting service, but there may be disadvantages that I haven't considered. You say there might be advantages on opening/closing connections? I'd be interested to hear more about that (I could open a question if you want :) ).

Andy
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Fonts Typography

From novice to tech pro — start learning today.

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.