SQLite helper class

Question

I have recently implemented my SQLite helper class that supports SQLite in a memory class to be opened to not to be lost. Please review it and tell me if there is a coding problem and tell me what to do to prevent\fix it.

using System;
using System.Collections.Generic;
using System.Data;
using System.Data.SQLite;
using System.Diagnostics;
using System.Globalization;
using System.IO;
using System.Linq;

namespace SQLite
{
    public class SqLiteDatabase : IDisposable
    {
        private readonly SQLiteConnection _dbConnection;

        /// <summary>
        /// Default Constructor for SQLiteDatabase Class.
        /// </summary>
        public SqLiteDatabase()
        {
            _dbConnection = new SQLiteConnection("Data Source=default.s3db");
        }

        /// <summary>
        /// Single Param Constructor to specify the datasource.
        /// </summary>
        /// <param name="datasource">The data source. Use ':memory:' for in memory database.</param>
        public SqLiteDatabase(String datasource)
        {
            _dbConnection = new SQLiteConnection(string.Format("Data Source={0}", datasource));
        }

        /// <summary>
        /// Single Param Constructor for specifying advanced connection options.
        /// </summary>
        /// <param name="connectionOpts">A dictionary containing all desired options and their values.</param>
        public SqLiteDatabase(Dictionary<String, String> connectionOpts)
        {
            String str = connectionOpts.Aggregate("",
                                                  (current, row) =>
                                                  current + String.Format("{0}={1}; ", row.Key, row.Value));
            str = str.Trim().Substring(0, str.Length - 1);
            _dbConnection = new SQLiteConnection(str);
        }

        #region IDisposable Members

        public void Dispose()
        {
            if (_dbConnection != null)
                _dbConnection.Dispose();

            GC.Collect();
            GC.SuppressFinalize(this);
        }

        #endregion

        public bool OpenConnection()
        {
            try
            {
                if (_dbConnection.State == ConnectionState.Closed)
                    _dbConnection.Open();
                return true;
            }
            catch (Exception e)
            {
                Console.WriteLine("SQLite Exception : {0}", e.Message);
            }

            return false;
        }

        public bool CloseConnection()
        {
            try
            {
                _dbConnection.Close();
                _dbConnection.Dispose();
            }
            catch (Exception e)
            {
                Console.WriteLine("SQLite Exception : {0}", e.Message);
            }

            return false;
        }

        /// <summary>
        /// Gets the specified table from the Database.
        /// </summary>
        /// <param name="sql">The table to retrieve from the database.</param>
        /// <returns>A DataTable containing the result set.</returns>
        public DataTable GetDataTable(string sql)
        {
            var table = new DataTable();

            try
            {
                using (SQLiteTransaction transaction = _dbConnection.BeginTransaction())
                {
                    using (var cmd = new SQLiteCommand(_dbConnection) {Transaction = transaction, CommandText = sql})
                    {
                        using (SQLiteDataReader reader = cmd.ExecuteReader())
                        {
                            table.Load(reader);
                            transaction.Commit();
                        }
                    }
                }

                return table;
            }
            catch (Exception e)
            {
                Console.WriteLine("SQLite Exception : {0}", e.Message);
            }
            finally
            {
                table.Dispose();
            }

            return null;
        }

        /// <summary>
        /// Executes a NonQuery against the database.
        /// </summary>
        /// <param name="sql">The SQL to execute.</param>
        /// <returns>A double containing the time elapsed since the method has been executed.</returns>
        public double? ExecuteNonQuery(string sql)
        {
            Stopwatch s = Stopwatch.StartNew();

            try
            {
                using (SQLiteTransaction transaction = _dbConnection.BeginTransaction())
                {
                    using (var cmd = new SQLiteCommand(_dbConnection) {Transaction = transaction})
                    {
                        foreach (string line in new LineReader(() => new StringReader(sql)))
                        {
                            cmd.CommandText = line;
                            cmd.ExecuteNonQuery();
                        }

                        transaction.Commit();
                    }
                }
                s.Stop();

                return s.Elapsed.TotalMinutes;
            }
            catch (Exception e)
            {
                Console.WriteLine("SQLite Exception : {0}", e.Message);
            }

            return null;
        }

        /// <summary>
        /// Gets a single value from the database.
        /// </summary>
        /// <param name="sql">The SQL to execute.</param>
        /// <returns>Returns the value retrieved from the database.</returns>
        public string ExecuteScalar(string sql)
        {
            try
            {
                using (SQLiteTransaction transaction = _dbConnection.BeginTransaction())
                {
                    using (var cmd = new SQLiteCommand(_dbConnection) {Transaction = transaction, CommandText = sql})
                    {
                        object value = cmd.ExecuteScalar();
                        transaction.Commit();
                        return value != null ? value.ToString() : "";
                    }
                }
            }
            catch (Exception e)
            {
                Console.WriteLine("SQLite Exception : {0}", e.Message);
            }

            return null;
        }

        /// <summary>
        /// Updates specific rows in the database.
        /// </summary>
        /// <param name="tableName">The table to update.</param>
        /// <param name="data">A dictionary containing Column names and their new values.</param>
        /// <param name="where">The where clause for the update statement.</param>
        /// <returns>A boolean true or false to signify success or failure.</returns>
        public bool Update(String tableName, Dictionary<String, String> data, String where)
        {
            string vals = "";
            if (data.Count >= 1)
            {
                vals = data.Aggregate(vals,
                                      (current, val) =>
                                      current +
                                      String.Format(" {0} = '{1}',", val.Key.ToString(CultureInfo.InvariantCulture),
                                                    val.Value.ToString(CultureInfo.InvariantCulture)));
                vals = vals.Substring(0, vals.Length - 1);
            }
            try
            {
                ExecuteNonQuery(String.Format("update {0} set {1} where {2};", tableName, vals, where));
                return true;
            }
            catch (Exception e)
            {
                Console.WriteLine("SQLite Exception : {0}", e.Message);
            }

            return false;
        }

        /// <summary>
        /// Deletes specific rows in the database.
        /// </summary>
        /// <param name="tableName">The table from which to delete.</param>
        /// <param name="where">The where clause for the delete.</param>
        /// <returns>A boolean true or false to signify success or failure.</returns>
        public bool Delete(String tableName, String where)
        {
            try
            {
                ExecuteNonQuery(String.Format("delete from {0} where {1};", tableName, where));
                return true;
            }
            catch (Exception e)
            {
                Console.WriteLine("SQLite Exception : {0}", e.Message);
            }

            return false;
        }

        /// <summary>
        /// Inserts new data to the database.
        /// </summary>
        /// <param name="tableName">The table into which the data will be inserted.</param>
        /// <param name="data">A dictionary containing Column names and data to be inserted.</param>
        /// <returns>A boolean true or false to signify success or failure.</returns>
        public bool Insert(String tableName, Dictionary<String, String> data)
        {
            string columns = "";
            string values = "";
            foreach (var val in data)
            {
                columns += String.Format(" {0},", val.Key);
                values += String.Format(" '{0}',", val.Value);
            }
            columns = columns.Substring(0, columns.Length - 1);
            values = values.Substring(0, values.Length - 1);
            try
            {
                ExecuteNonQuery(String.Format("insert into {0}({1}) values({2});", tableName, columns, values));
                return true;
            }
            catch (Exception e)
            {
                Console.WriteLine("SQLite Exception : {0}", e.Message);
            }

            return false;
        }

        /// <summary>
        /// Wipes all the data from the database.
        /// </summary>
        /// <returns>A boolean true or false to signify success or failure.</returns>
        public bool WipeDatabase()
        {
            DataTable tables = null;

            try
            {
                tables = GetDataTable("select NAME from SQLITE_MASTER where type='table' order by NAME;");
                foreach (DataRow table in tables.Rows)
                {
                    WipeTable(table["NAME"].ToString());
                }
                return true;
            }
            catch (Exception e)
            {
                Console.WriteLine("SQLite Exception : {0}", e.Message);
            }
            finally
            {
                if (tables != null) tables.Dispose();
            }

            return false;
        }

        /// <summary>
        /// Wipes all the data from the specified table.
        /// </summary>
        /// <param name="table">The table to be wiped.</param>
        /// <returns>A boolean true or false to signify success or failure.</returns>
        public bool WipeTable(String table)
        {
            try
            {
                ExecuteNonQuery(String.Format("delete from {0};", table));
                return true;
            }
            catch (Exception e)
            {
                Console.WriteLine("SQLite Exception : {0}", e.Message);
            }

            return false;
        }
    }
}

Answer 1

In Dispose():

1. You are calling GC.Collect() which isn't necessarily need unless it is very important to collect all inaccessible memory when disposing.
2. GC.SuppressFinalize(this) isn't need because the class don't have a finalizer/deconstructor

In CloseConnection() you are always returning false. Why not make it void?

You are handling a lot of exceptions in this "framework", why not let the "client" handle the errors? It will make you code more maintainable and easy to read. It would remove a bunch of return and try..catch statements (as discussed in the comments), and allows the "client" to handle the exceptions differently if needed.

Answer 2

It looks to be a reasonable design and I am reworking mine a bit based on some ideas I have seen here. But I don't like the idea of not being able to unit test, which this design would not allow for.

I also do not like to see hardcoded values in the helper class. Those should come in from the outside. I do agree with the other posts that are saying not to do the try catch here, as it hides it from the call code. At this level in a framework, you'd not have try/catch blocks and you let the errors simply roll up.

I'd recommend adding dependency injection. Then in your application that uses this framework, simply define the objects there, and everything will pass in. This will allow you to mock the class and add true unit tests. Currently you cannot do that.

Also, if you wanted to use this helper for an application with multiple database files, you'd have to create multiple helpers like this. A better approach would be to pass it in through construction injection, and allow the helper to be used with any database.

Also, I am not sure what the point is of a transaction on the GetDataTable() method. When you commit this, what is being committed? It is a read-only process.

Here's my winForm code I used to setup the database. This is the only place in the code where the database is defined. If I wanted a second database as well, I could simply create a second _dbConnection object using a different name.

I am using a DAO class per database I am connecting to. So my form talks to the DAO, my DAO is the only way to get to the SQLite helper class. I setup the db connection in the form, and pass it in. That way I can mock the DAO class for unit testing without dependencies on the database itself. I have not, but I will implement interfaces as needed. But I still need more time to work through this is bit.

So this is my current approach after about 12-hours of working with SQLite. So take that for what it is worth, as I am new to SQLite.

UPDATE: As I was writing this I desided to change my connection to just a connection string, that way my forms could work with Access, SQLite, or SQL Server by just using different DAO or Helper classes.

    private readonly SQLiteConnection _dbConnection = new SQLiteConnection("Data Source=LanguageTutor.db;Version=3;New=False;Compress=True;");        
    private readonly Dao _dao;

    public Form2()
    {
        InitializeComponent();
        _dao = new Dao(_dbConnection);
    }

Then my DAO class

    public class Dao
    {

    private readonly SqLiteHelper _sql;
    private readonly SQLiteConnection _dbConnection;

    public Dao(SQLiteConnection dbConnection)
    {
        _dbConnection = dbConnection;
        _sql = new SqLiteHelper(_dbConnection);
    }

Now here's how I reworked my own SQLite helper class. This just covers the constructors and dependency injection and I took the dictionary one from you as well.

    private readonly SQLiteConnection _dbConnection; 

    #region Constructors

    /// <summary>
    /// Default Constructor for SQLiteDatabase Class.
    /// </summary>
    public SqLiteHelper(SQLiteConnection sqLiteConnection)
    {
        _dbConnection = sqLiteConnection;
    }

    public SqLiteHelper(SQLiteConnection sqLiteConnection, Dictionary<String, String> connectionOpts)
    {
        String str = connectionOpts.Aggregate("", (current, row) => current + String.Format("{0}={1}; ", row.Key, row.Value));
        str = str.Trim().Substring(0, str.Length - 1);

        _dbConnection = sqLiteConnection;
        _dbConnection.ConnectionString = str;
    }

    #endregion

Answer 3

IDisposable implementation

Implementing IDisposable is a little more complicated than just Dispose(). You will want to override at least the protected virtual void Dispose(bool disposing) method. For details see Jon Skeet's answer here: https://stackoverflow.com/questions/574019/calling-null-on-a-class-vs-dispose/574659#574659

Exception handling

I personally dislike bool return values to report success or error. It makes me write:

using (SqLiteDatabase db = new SqLiteDatabase())
{
    if (!db.OpenConnection())
        // handle error
        return;

    DataTable table = db.GetDataTable("table");
    if (table == null)
        // handle error
        return;

    string value = db.ExecuteScalar(statement);
    if (value == null)
        // handle error
        return;

    if (!db.Update(...))
        // handle error
        return
}

Not only the error conditions are inconsistent (sometimes false is returned, other times null), it's quite easy to ignore the return values.

I prefer

try
{
    using (SqLiteDatabase db = new SqLiteDatabase())
    {
        db.OpenConnection();
        DataTable table = db.GetDataTable("table");
        string value = db.ExecuteScalar(statement);
        db.Update(...)
    }
}
catch (SqLite.FailedToOpenDatabaseException e)
{
    // handle
}
catch (SqLite.TableDoesNotExistException e)
{
    // handle
}
catch (SqLite.FailedToUpdateTable e)
{
    // handle
}

(given that Dispose closes the connection)

Loose ends

• I would't include timing/profiling in ExecuteNonQuery as it does not really fit with the rest of the interface.