Question

I seemingly have two choices:

  1. Make my class implement IDisposable. Create my DbCommand instances as private readonly fields, and in the constructor, add the parameters that they use. Whenever I want to write to the database, bind to these parameters (reusing the same command instances), set the Connection and Transaction properties, then call ExecuteNonQuery. In the Dispose method, call Dispose on each of these fields.
  2. Each time I want to write to the database, write using(var cmd = new DbCommand("...", connection, transaction)) around the usage of the command, and add parameters and bind to them every time as well, before calling ExecuteNonQuery. I assume I don't need a new command for each query, just a new command for each time I open the database (right?).

Both of these seem somewhat inelegant and possibly incorrect.

For #1, it is annoying for my users that I this class is now IDisposable just because I have used a few DbCommands (which should be an implementation detail that they don't care about). I also am somewhat suspicious that keeping a DbCommand instance around might inadvertently lock the database or something?

For #2, it feels like I'm doing a lot of work (in terms of .NET objects) each time I want to write to the database, especially with the parameter-adding. It seems like I create the same object every time, which just feels like bad practice.

For reference, here is my current code, using #1:

using System;
using System.Net;
using System.Data.SQLite;

public class Class1 : IDisposable
{
    private readonly SQLiteCommand updateCookie = new SQLiteCommand("UPDATE moz_cookies SET value = @value, expiry = @expiry, isSecure = @isSecure, isHttpOnly = @isHttpOnly WHERE name = @name AND host = @host AND path = @path");
    public Class1()
    {
        this.updateCookie.Parameters.AddRange(new[]
                            {
                                new SQLiteParameter("@name"),
                                new SQLiteParameter("@value"),
                                new SQLiteParameter("@host"),
                                new SQLiteParameter("@path"),
                                new SQLiteParameter("@expiry"),
                                new SQLiteParameter("@isSecure"),
                                new SQLiteParameter("@isHttpOnly")
                            });
    }

    private static void BindDbCommandToMozillaCookie(DbCommand command, Cookie cookie)
    {
        long expiresSeconds = (long)cookie.Expires.TotalSeconds;

        command.Parameters["@name"].Value = cookie.Name;
        command.Parameters["@value"].Value = cookie.Value;
        command.Parameters["@host"].Value = cookie.Domain;
        command.Parameters["@path"].Value = cookie.Path;
        command.Parameters["@expiry"].Value = expiresSeconds;
        command.Parameters["@isSecure"].Value = cookie.Secure;
        command.Parameters["@isHttpOnly"].Value = cookie.HttpOnly;
    }

    public void WriteCurrentCookiesToMozillaBasedBrowserSqlite(string databaseFilename)
    {
        using (SQLiteConnection connection = new SQLiteConnection("Data Source=" + databaseFilename))
        {
            connection.Open();
            using (SQLiteTransaction transaction = connection.BeginTransaction())
            {
                this.updateCookie.Connection = connection;
                this.updateCookie.Transaction = transaction;

                foreach (Cookie cookie in SomeOtherClass.GetCookieArray())
                {
                    Class1.BindDbCommandToMozillaCookie(this.updateCookie, cookie);
                    this.updateCookie.ExecuteNonQuery();
                }

                transaction.Commit();
            }
        }
    }

    #region IDisposable implementation
    protected virtual void Dispose(bool disposing)
    {
        if (!this.disposed && disposing)
        {
            this.updateCookie.Dispose();
        }
        this.disposed = true;
    }
    public void Dispose()
    {
        this.Dispose(true);
        GC.SuppressFinalize(this);
    }
    ~Class1()
    {
        this.Dispose(false);
    }

    private bool disposed;
    #endregion
}
Was it helpful?

Solution

Domenic, Disposing a SQLiteCommand simply signals its active reader to dispose it's reader and sets the parameters and connection reference to null.

You are not at risk of resource leak by caching a command as long as you properly dispose of the executed reader and close/dispose of the connection.

So, reusing a cached command and simply assigning values to the parameters is by far the most efficient implementation.

.Prepare() is a noop in SQLiteCommand so there is no benefit to be had there.

OTHER TIPS

There's no single "right way" to handle your database object life cycle. It's all dependent on your application needs.

My personal preference is to keep the code as simple as possible. I tend to recreate Command and Parameter objects as needed. This lets my functions be as self contained as possible. It has greatly simplified any re factoring I've had to do.

If you're concerned about the performance hit of recreating objects you should load up a profiler and see where your bottle necks are. In applications I've built, I've found the time spent creating DbCommand objects is so inconsequential compared to the time spent executing the query that it doesn't really factor in to my apps performance.

I have found with sqlserver if we reuse the command with same connection, its much faster. I plan to test for different connections and transactions but they appear to be faster.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top