Singleton design pattern and DB connection 2

[rjoubert]

I've just implemented a Singleton class to handle my Access DB connection. My app was having a problem with database calls taking too long to execute. I have several areas where I autogenerate data, and for each call I was instantiating a DB connection, opening it, performing a DB insert, and closing the connection. This was very inefficient, so I decided to work on the Singleton solution. Here's the code for my class...

Public Class clsDBConnect

    Private Shared s_objDBConnect As clsDBConnect
    Private Shared s_objConnection As OleDbConnection

    Protected Sub New()

        s_objConnection = New OleDbConnection
        s_objConnection.ConnectionString = My.Settings.MyConnectionString
        s_objConnection.Open()

    End Sub

    Public Shared Function GetInstance() As clsDBConnect

        If s_objDBConnect Is Nothing Then
            s_objDBConnect = New clsDBConnect
        End If

        Return s_objDBConnect

    End Function

    Public ReadOnly Property GetConnection() As OleDbConnection
        Get
            Return s_objConnection
        End Get
    End Property

End Class

And here's how I'm utilizing the Singleton class when making DB calls...

Public Sub CreateParentRecord(Param1, Param2, Param3)

   Dim objDBConnect As clsDBConnect
   Dim objCommand As OleDbCommand
   Dim objConnection As OleDbConnection

   objDBConnect = clsDBConnect.GetInstance
   objCommand = New OleDbCommand
   objConnection = objDBConnect.GetConnection

   With objCommand
      .CommandText = "INSERT INTO MyTable (Field1, Field2, Field3) SELECT " & Param1 & ", " & Param2 & ", " & Param3

      .Connection = objConnection
      .ExecuteNonQuery()

   End With

   For i = 1 to 12 
      CreateChildRecord(i)
   Next

End Sub

Public Sub CreateChildRecord(Param1)
   ' similar DB code as the CreateParentRecord procedure, just different command text
End Sub

If you notice, I'm opening the connection within the New procedure of the Singleton class, and I'm not closing it anywhere. Where should I be closing and cleaning up my connection? Keep in mind that I want to be able to keep the same connection open for the creation of the parent record and the 12 child records. Any help or suggestions would be greatly appreciated...thanks.

 

[AlexCuse]

Have your class implement IDisposable (I guess you don't really NEED to do this part), but then in your Dispose() method you can close and dispose of your DB connection.

You might try opening and closing your connection within your methods so that it doesn't remain open, and see if you still have the performance hit. Keeping DB connections open is generally not a good idea if you can avoid it.

hope this helps,

Alex

[small]----signature below----[/small]
Majority rule don't work in mental institutions

My Crummy Web Page

 

[AlexCuse]

Also, you may find this useful if you want to be able to use your class as a non-singleton as well (plenty of places to convert it to C#)

using System;
using System.Collections.Generic;
using System.Text;

namespace Utilities
{
    /// <summary>
    /// Provides a singleton instance of any type (must have default constructor)
    /// </summary>
    /// <typeparam name="T">Type we want to create an instance of</typeparam>
    public sealed class SingletonProvider<T> where T : new()
    {
        SingletonProvider() { }

        /// <summary>
        /// Returns a new Instance of T
        /// </summary>
        public static T Instance
        {
            get { return SingletonCreator.instance; }
        }

        /// <summary>
        /// Ensures that instance is a Singleton
        /// </summary>
        class SingletonCreator
        {
            static SingletonCreator() { }

            internal static readonly T instance = new T();
        }

    }
}

This saves a lot of pain in coding your individual classes as Singletons.

You use it like SingletonProvider<MyClass>.Instance

And this: objDBConnect = clsDBConnect.GetInstance is probably unnecessary, you don't want to create too many references if you can avoid it. You could just set connection to csDBConnect.GetInstance directly.



[small]----signature below----[/small]
Majority rule don't work in mental institutions

My Crummy Web Page

 

[rjoubert]

Alex, thanks for the replies.

Have your class implement IDisposable (I guess you don't really NEED to do this part)...

Can you tell me why you said I wouldn't really NEED to do that?

I tried opening and closing the connection within my methods, but that brought back the big performance hit. I agree with you that it's probably not a good idea to keep it open, which is why I posted this to begin with.

It would be very easy for me to open ONE connection within CreateParentRecord method and use that same connection for the 12 calls to CreateChildRecord. But the issue there is that parts of my application may just call CreateChildRecord without calling the CreateParentRecord method. So just about every data access method I have has to be able to open the DB connection by itself. I hope everyone can get the gist of what I'm trying to say here...it's late on a Friday and my brain is already in weekend mode, so it isn't exactly firing on all cylinders right now.

 

[AlexCuse]

I figured that might happen. Access connections are kinda slow sometime.

What I meant with that statement is you don't necessarily need to implement IDisposable, but you do need to have a "Dispose" type of method to handle closing/disposing of that connection when your done with the object.

Does that make sense?

[small]----signature below----[/small]
Majority rule don't work in mental institutions

My Crummy Web Page

 

[Craig0201]

Why is this a singleton? Is it REALLY something which must be unique?

I doubt it, personally.

C

 

[AlexCuse]

If there is a continually open database connection involved, I can see why you would want to make sure there is only one of 'em.

[small]----signature below----[/small]
Majority rule don't work in mental institutions

My Crummy Web Page

 

[Craig0201]

But this is just hiding the problem.

The 'keep open for 12 if master insert' but 'open separately if not' could be resolved by overloading, not by keeping a db connection open.

C

 

[AlexCuse]

I'd tend to agree, but without knowing the amount of effort required of rjoubert to make this change, or the way his application is currently designed, I don't think that is my call to make.

[small]----signature below----[/small]
Majority rule don't work in mental institutions

My Crummy Web Page

 

[rjoubert]

Basically I want to open a DB connection if one is not already opened, or use the currently open DB connection if it's already open. That's why I started down the Singleton pattern...plus it's a simple pattern to implement, and I'm all about simple these days.

Craig...could you please elaborate on your overloading idea?

 

[AlexCuse]

this looks like it might be a good read:

http://www.devx.com/dotnet/Article/9303

Basically you will want to pass your already open connection as a parameter to your "CreateChildRecord" function in certain cases (IE when the sub is being called from CreateParentRecord), and use that connection to execute the necessary commands (so you don't need to get a new connection with each call).

Make sense?

It'd look something like this:

Public Sub CreateParentRecord(Param1, Param2, Param3)

   Dim objDBConnect As clsDBConnect
   Dim objCommand As OleDbCommand
   Dim objConnection As OleDbConnection

   objDBConnect = clsDBConnect.GetInstance
   objCommand = New OleDbCommand
   objConnection = objDBConnect.GetConnection

   With objCommand
      .CommandText = "INSERT INTO MyTable (Field1, Field2, Field3) SELECT " & Param1 & ", " & Param2 & ", " & Param3

      .Connection = objConnection
      .ExecuteNonQuery()

   End With

   For i = 1 to 12 
      CreateChildRecord(i)
   Next

End Sub

Public [b]Overloads[/b] Sub CreateChildRecord(Param1)
   ' similar DB code as the CreateParentRecord procedure, just different command text
End Sub

[b]Public Overloads Sub CreateChildRecord(Param1, Param2 [COLOR=red]as IDbConnection[/color])
    ' this overload will execute the command using the connection that is passed it
End Sub[/b]

Hope this helps,

Alex

[small]----signature below----[/small]
Majority rule don't work in mental institutions

My Crummy Web Page

 

[rjoubert]

Yes indeed that makes sense...thank you. One more question...should I be passing that connection parameter By Val or By Ref?

 

[Craig0201]

rjoubert,

That's exactly what I had in mind. You should pass by ref. This is an object type, not a value type.

You can then open and close the connections and remove the "false" singleton.

C

 

[rjoubert]

Great...thanks to both of you for your help. Now I just have to go through my DB access code and find more places to implement this.

 

[AlexCuse]

rjoubert - I found myself having a similar problem today. I don't do a ton of work writing back to access db's but running a batch of queries was very slow because of the opening/closing of connections.

I added an overloaded method that uses an enum to control whether its the first, middle, or last execution of a batch. It also wraps all of the action queries in a transaction, which is something I forgot to mention. So it allows me to do something like loop over a collection of objects from the calling code and execute a query for each one, while still maintaining the ability to take advantage of opening the connection only once, and using a transaction. It does get a little strange (first object, you need to pass one enum value, then another for all the objects in the middle, and a third value for the last), but it reduced execution time on relatively small datasets by a factor of 8-10. The loop I was timing included address standardization calls to another assembly as well, so the access connection and lack of transaction use were clearly the biggest culprits.

I immediately thought of your problem, and thought you might find it of use.

public void RunSQL(String sql, Enums.TransactionState tranState)
{
    if (tranState == Enums.TransactionState.Begin)
    {
        _connDb.Open();
        _tran = _connDb.BeginTransaction();
    }

    OleDbCommand cmd = new OleDbCommand(sql, _connDb, _tran);

    try
    {
        cmd.ExecuteNonQuery();
    }
    catch (Exception ex)
    {
        throw ex;
    }
    finally
    {
        if (tranState == Enums.TransactionState.Commit)
        {
            _tran.Commit();
            _connDb.Close();
            _tran = null;
        }


        if (null != cmd)
        {
            cmd.Dispose();
        }
    }
}

The enum was just:

public enum TransactionState
{
    /// <summary>
    /// Open a new transaction
    /// </summary>
    Begin,
    /// <summary>
    /// Continue using open transaction
    /// </summary>
    Continue,
    /// <summary>
    /// Commit transaction when finished
    /// </summary>
    Commit
}

It's a little more complex to call, but I think its' worth it.

Hope it helps,

Alex

[small]----signature below----[/small]
Majority rule don't work in mental institutions

My Crummy Web Page

 

[rjoubert]

Alex...that's a great idea, but what if you are only running one command (so the Transaction state needs to be both a begin and end)? You may not be doing that in your code, but I have a few spots where I'm only running one command.

I've modified my code to open the connection and pass that same connection into various subprocedures. And I've also deleted the Singleton class. One procedure that used to take over 90 seconds now takes about 3 seconds to complete.

 

[AlexCuse]

If I'm only running one command then I use

public void RunSQL(String sql)

;-)

I added that one just to allow me to control batching when I'm looping over a collection

[small]----signature below----[/small]
Majority rule don't work in mental institutions

My Crummy Web Page

 

[Sorwen]

I assume you have multiple users (or web/programs) working on the same data and they need to know the data has been updated immediately? If that isn't the case you should never open/close your database that much. Especially with Access/Jet that loves to corrupt databases on a whim.

If you do have an update immediately scenario then they have you on the right track of passing the connection if it is open. If that isn't the case you should really persist your data in another way and update after you've complete all the little things that had you opening/closing you database.

-I hate Microsoft!
-Forever and always forward.
-My kingdom for a edit button!

 

[rjoubert]

Sorwen...yes, I have multiple users working on the same data. Access is certainly not my 1st choice as the DB, but we're a bit limited here. We are working on securing a SQL Server DB and moving this application over to that DB platform. I just hope it happens sooner rather than later.

 

[gmmastros]

rjoubert,

SQL Server Express is a free version of Microsoft SQL Server 2005. SQL Server Express can be installed on most operating systems, from desktops to servers.

There are certain limitations with Access. Oddly enough, those limitations are similar to the limitations that exist for SQL Server Express.

When you need to upgrade to a 'beefier' version of SQL Server, you simply detach the database from your SQL Express instance and attach it to your SQL Server 2005 instance.

I understand about limits (within an organization). SQL Server Express is much better than Access, and it's free. The point I am trying to make is.... don't let financial limitations prevent you from using SQL Server.

>> I just hope it happens sooner rather than later.

No time like the present.

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom