CODEWITHSUNDEEP
c#asp.net-mvcodbcdata-access-layerdispose
leora
leora

Reputation: 196689

What is the right pattern to close an OdbcConnection in an asp.net-mvc web page

I have a website and I have a data access class that has an OdbcConnection. In the constructor I have this code:

    public MySybaseProvider()
    {
        _conn = GetConn();        
    }

    public OdbcConnection GetConn()
    {
        string connString = "DSN=SybaseIQ;Eng=SYSERVER;Links=tcpip(Host=" + _connectionInfo.Host + ";Port=" + _connectionInfo.Port + ");UID=" + _connectionInfo.User + ";PWD=" + _connectionInfo.Pwd + ";";

        return new OdbcConnection(connString);
    }

and through out the class i have the following code:

    private OdbcQuery GetQuery(string sql)
    {
        return new OdbcQuery(_conn, sql);
    }

I am trying to figure out the best way to make sure I close the connection properly. Should I implement IDisposable? should i open and close the connection on each query? Are there any other best practices here.

EDIT:

To clarify, my use case is that i am loading a single web page but it takes about 10 queries to get all of the data needed for the page.

Upvotes: 1

Views: 1624

Answers (4)

Darren
Darren

Reputation: 329

So I have submitted another Answer Because I realised that I usually use Enterprise Library for my Data Access and not pure .net which changes things a bit. For one the DataAccessApplicationBlock (DAAB) takes care of opening and closing the Connection(s) to the Database so I don't have too, therefore I use it like below. Also in this case a easier and more effiecint way to do what is described is to execute all the SELECT Statements in 1 Stored Proc and Then Read through and Parse each result set one at a time.

Example:

Stored Proc

Create PROCEDURE [dbo].[GetUserByEmail]
    @UserID int,
    @AccountID int,
AS
BEGIN
    SET NOCOUNT OFF;

    SELECT * FROM Users WHERE UserID = @UserID and AccountID = @AccountID

    SELECT * FROM UserAddresses WHERE UserID = @UserID
END

C# Code:

public class DataAccessLayer
{
    private static DataAccessLayer me = new DataAccessLayer();

    private DataAccessLayer() { }

    public static DataAccessLayer GetInstance()
    {
        return me;
    }

    public Database GetDatabase(string connectionString, string provider)
    {
       DbProviderFactory providerFactory = DbProviderFactories.GetFactory(provider);

        return new GenericDatabase(connectionString,
                                   providerFactory);
    }
}

public class Repository
{
    protected Database curDatabase;

    public Repository()
    {
        curDatabase = DataAccessLayer.GetInstance().GetDatabase("ConnectionString", "System.Data.Odbc");
    }

    public Database CurrentDatabase
    {
        get { return curDatabase; }
    }
}

public class UserRepository : Repository
{
    public User GetUser(int urserID, int accountID)
    {
        using (IDataReader dataReader = CurrentDatabase.ExecuteReader("StroedProcName", urserID, accountID))
        {
            //User Details
            while (dataReader.Read())
            {
                //Parse Data
            }

            dataReader.NextResult();

            //User Address(es)
            while (dataReader.Read())
            {
                //Parse Data
            }
        }
    }
}

Upvotes: 1

Jordão
Jordão

Reputation: 56497

Check if ODBC connection pooling is enabled, and create and open a connection object with the using statement for each of your accesses to the database:

using (var conn = new OdbcConnection(_connString)) {
  conn.Open();

  // do a database command ...
}

(Note that I changed the design to store your _connString in a field)

This is the most idiomatic way to do this, and you don't need to worry about creating your connection in the constructor, storing it in a field, or even implementing IDisposable.

Upvotes: 1

tsells
tsells

Reputation: 2771

You can use the following pattern to ensure the connection is disposed. Even if an error occurs it will be disposed of. For safer coding you should use a connection string builder to build the connection. You should only create / use the connection when you are planning to perform an operation. It should NOT hang around for the life of your application. You can use the connection string / properties for that instead. 

    public OdbcConnection GetConn()
    {
        OdbcConnectionStringBuilder sb = new OdbcConnectionStringBuilder();
        sb.Driver = "Microsoft Access Driver (*.mdb)";
        sb.Add("Dbq", "C:\\info.mdb");
        sb.Add("Uid", "Admin");
        sb.Add("Pwd", "pass!word1");
        OdbcConnection con = new OdbcConnection(sb.ConnectionString);
        return con;
    }


    public void DoSomeWork()
    {
        using(OdbcConnection connection = GetConn())
        {
            // Do stuff with your connection here


        }
    }

Upvotes: 0

Matt Greer
Matt Greer

Reputation: 62057

I would argue you definitely should implement IDisposable and follow the dispose pattern Keep in mind the garbage collector knows nothing about IDisposable. You should as a last resort have a finalizer that will kill the DB connection if need be, but suppress the finalizer whenever possible (see the link I added for all the gory details).

As to whether you want to create a new connection per command or keep one around for the life of the object, that's much more subjective and depends on your scenario, environment, and many other things. I recommend exploring both and see what you think. If this is a "real" DB, perhaps talk to the DBA and see what they think.

Either way, implementing IDisposable is your friend, as you can have nice, clean, well defined disposing code in a pattern that all .NET developers are familiar with.

Upvotes: 0

Related Questions