2

I have a C# application that has been running just fine for the past year. However, in the past couple of months we have added more clients and the database has been reporting connection errors on a more consistent basis. It might be time to revisit the way connections are handled and perhaps there is a better way to do this. Here is the class that manages my connections:

public class myDAL
    {
        protected SqlConnection sqlConnection = new SqlConnection();

        protected void openConnection(string connection)
        {
                sqlConnection.ConnectionString = connection;
                sqlConnection.Open();            
        }


        protected void closeConnection()
        {            
            sqlConnection.Close();
        }

}

Note that I'm really doing nothing special to manage the connection. I just call the open and close as needed and this is happening from multiple clients at the same time. Am I doing anything obviously wrong here?

Unknown Coder
  • 6,625
  • 20
  • 79
  • 129
  • 1
    What are the actual errors? Do you only see error's when connecting, i.e. when calling `openConnection`. – Ash Burlaczenko Sep 19 '11 at 18:00
  • Retry logic could be used to try and open the connection (again) if it fails on the initial attempt. – Jon Raynor Sep 19 '11 at 18:03
  • Sorry, I meant to include the error. The errors are variations on the transport errors with the most common of: A network-related or instance-specific error occurred while establishing a connection to SQL Server. The server was not found or was not accessible. Verify that the instance name is correct and that SQL Server is configured to allow remote connections. (provider: Named Pipes Provider, error: 40 - Could not open a connection to SQL Server) – Unknown Coder Sep 19 '11 at 18:11
  • Are you sure your database server isn't simply overloaded? Check its CPU load. – Max Sep 19 '11 at 18:20

5 Answers5

3

Jim your practice of having open and close connection methods inside another class is very old, modern .NET development follows a patter like this nowadays:

using (SqlConnection conn = new SqlConnection("connection string here"))
using (SqlCommand cmd = new SqlCommand("sql query", conn))
{
    // execute it blah blah
}

see here: Closing SqlConnection and SqlCommand c# or search in SO for hundreds of questions and answers all telling the same, close the connection immediately either with a using like in this example or with a try/finally inside the same method, no need for one method to open and on method to close it, just prone to errors if anything happens in the between.

Community
  • 1
  • 1
Davide Piras
  • 43,984
  • 10
  • 98
  • 147
  • "modern .NET development follows...". Thankfully, it is not mandatory to follow "modern" patterns, just for the shake of being "modern", but only when there is an actual gain in it, e.g. in performance, code maintance, execution speed etc. – ThunderGr Dec 13 '12 at 11:53
1

Use IDisposable Interface on your class, Also use Disconnected Data that will be beneficial

public class myDAL:IDisposable
 {
    protected SqlConnection sqlConnection = new SqlConnection();
    protected void openConnection(string connection)
    {
            sqlConnection.ConnectionString = connection;
            sqlConnection.Open();            
    }


    protected void closeConnection()
    {            
        sqlConnection.Close();
    }

    public void Dispose()
    {
        sqlconnection.Close();
       //Dispose of the connection
    }

 } 

use the statement

using (MyDal Conn= new MyDal())
{
  //Code 
}
Nivid Dholakia
  • 5,272
  • 4
  • 30
  • 55
0

I believe that what you want has been answered here. Implementing a retrying procedure is essential when you expect a lot of traffic to the DB. In that question, the focus is on deadlocks, but it is very useful with all sorts of SQL Exceptions as well, including the connection exceptions.

This may be more than an year late but, it can be useful to other people that want an answer to this question.

Community
  • 1
  • 1
ThunderGr
  • 2,297
  • 29
  • 20
0

You could implement a IDisposable interface in a wrapper class.

And/Or

With the using statement open and close your connection.

using(var db = new SqlConnection()
{
  db.Something();
}

Now the connection is closed automatically.

When do you close your connection? After each batch of commands or after a interval?

Max
  • 1,068
  • 1
  • 10
  • 15
0

Depending on how frequently the calls are made I would consider something like:

  using (var sqlconnection = new SqlConnection())
  {
  }

or if that's not fitting the task you can maybe append your DAL with something like (just pseudo-example):

 protected void SqlExecute(Action a)
 {
   if (sqlConnection.State != ConnectionState.Connected)
     sqlConnection.Open();

   a();
 }

and in your call:

       SqlExecute(() =>
                       {
                           DoSth();
                       });
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
David
  • 2,551
  • 3
  • 34
  • 62
  • Then there will be no call to CloseConnection. – Max Sep 19 '11 at 18:09
  • @Job That's why i am stating it's "pseudo"... In a scenario where u wouldn't like to loose the ms re-opening the connection for example, there could be an additional parameter like "bool keepConnectionOpen" – David Sep 19 '11 at 18:21