2
\$\begingroup\$

I want to move this to a class that will open an sqlite connection that can be re-used, rather than disposing of it every time I write to the database. Furthermore I am more than happy for any best practices from sqlite users! I am aware of a similar thread and am trying to absorb it:

SQLite helper class

void createDB()
{
    // Dont forget to del if refreshing
    if (!File.Exists(connstring))
    {
        SQLiteConnection.CreateFile(connstring);
        SQLiteConnection sqliteCon = new SQLiteConnection(@"Data Source=" + connstring);
        sqliteCon.Open();

        // Define db structure
        string createfilesTableSQL = "CREATE TABLE files ( id INTEGER PRIMARY KEY, filename TEXT, creationtime TEXT, lastwrite TEXT, lastaccess TEXT, checksum TEXT);";
        string createfoldersTableSQL = "CREATE TABLE folders ( id INTEGER PRIMARY KEY, dirname TEXT, creationtime TEXT, lastwrite TEXT, lastaccess TEXT, checksum TEXT);";

        using (SQLiteTransaction sqlTransaction = sqliteCon.BeginTransaction())
        {
            // Create the tables
            SQLiteCommand createCommand = new SQLiteCommand(createfilesTableSQL, sqliteCon);
            createCommand.ExecuteNonQuery();
            createCommand.Dispose();

            SQLiteCommand createCommand2 = new SQLiteCommand(createfoldersTableSQL, sqliteCon);
            createCommand2.ExecuteNonQuery();
            createCommand2.Dispose();

            // Commit the changes into the database
            sqlTransaction.Commit();
        } // end using

        // Close the database connection
        sqliteCon.Close();
        sqliteCon.Dispose();
    }
}
public int fileExists(string filename, string lastwrite, string hash)
{
    /* Here is a quick Map
     0 "The file does not exist in the database
     1 "The Hash Matches, leave it alone
     2 "the file is newer than that of the databse
     3 "the file is older than that of the databse"
     4 "the file is the same as that of the database"
     */

    // Connect to database
    SQLiteConnection sqliteCon = new SQLiteConnection(@"Data Source=" + connstring);
    sqliteCon.Open();

    //Query
    //string selectSQL = "SELECT EXISTS(SELECT 1 FROM files WHERE filename=" + filename + " LIMIT 1) as result;";
    string selectSQL = "SELECT * FROM files WHERE filename=" + filename + ";";
    SQLiteCommand selectCommand = new SQLiteCommand(selectSQL, sqliteCon);
    SQLiteDataReader dataReader = selectCommand.ExecuteReader();

    //does file exist
    bool exists = dataReader.Read();

    if (!exists)
    {
        dataReader.Dispose();
        sqliteCon.Dispose();
        return 0; //it doesnt exist in the database
    }

    string dbhash = dataReader["checksum"].ToString();
    string dblastwrite = dataReader["lastwrite"].ToString();
    string lastwritex = lastwrite.Replace("'", "");
    string hashx = hash.Replace("'", "");
    DateTime dblastwriteDT = Convert.ToDateTime(dblastwrite);
    DateTime lastwriteDT = Convert.ToDateTime(lastwritex);

    dataReader.Dispose();
    sqliteCon.Dispose();


    if (dbhash == hashx)
    {
        return 1; //hash matches the database version
    }

    if (lastwriteDT > dblastwriteDT)
    {
        return 2; //the file is newer than that of the databse
    }

    if (lastwriteDT < dblastwriteDT)
    {
        return 3; //the file is older than that of the databse
    }
    else
    {
        return 4; //the file is the same as that of the database
    }
}
void cleanDB()
{
    SQLiteConnection sqliteCon = new SQLiteConnection(@"Data Source=" + connstring);
    sqliteCon.Open();

    // Define db structure
    string createfilesTableSQL = "delete from files;";
    string createfoldersTableSQL = "delete from folders;";

    using (SQLiteTransaction sqlTransaction = sqliteCon.BeginTransaction())
    {
        // Create the tables
        SQLiteCommand createCommand = new SQLiteCommand(createfilesTableSQL, sqliteCon);
        createCommand.ExecuteNonQuery();
        createCommand.Dispose();

        SQLiteCommand createCommand2 = new SQLiteCommand(createfoldersTableSQL, sqliteCon);
        createCommand2.ExecuteNonQuery();
        createCommand2.Dispose();

        // Commit the changes into the database
        sqlTransaction.Commit();
    } // end using

    // Close the database connection
    sqliteCon.Dispose();
}
private void addFile(string filename, string creationtime, string lastwrite, string lastaccess, string checksum)
{
    // Open connection to database
    SQLiteConnection sqliteCon = new SQLiteConnection(@"Data Source=" + connstring);
    sqliteCon.Open();
    using (SQLiteTransaction SQLiteTrans = sqliteCon.BeginTransaction())
    {
        using (SQLiteCommand cmd = sqliteCon.CreateCommand())
        {
            // Insert a new file record
            cmd.CommandText = "INSERT INTO files(filename, creationtime, lastwrite, lastaccess, checksum)" + " VALUES (?,?,?,?,?)";

            SQLiteParameter Field1 = cmd.CreateParameter();
            SQLiteParameter Field2 = cmd.CreateParameter();
            SQLiteParameter Field3 = cmd.CreateParameter();
            SQLiteParameter Field4 = cmd.CreateParameter();
            SQLiteParameter Field5 = cmd.CreateParameter();

            cmd.Parameters.Add(Field1);
            cmd.Parameters.Add(Field2);
            cmd.Parameters.Add(Field3);
            cmd.Parameters.Add(Field4);
            cmd.Parameters.Add(Field5);
            Field1.Value = filename;
            Field2.Value = creationtime;
            Field3.Value = lastwrite;
            Field4.Value = lastaccess;
            Field5.Value = checksum;

            cmd.ExecuteNonQuery();
        }

        SQLiteTrans.Commit();
    }
    sqliteCon.Dispose();
}
private void addFolder(string dirname, string creationtime, string lastwrite, string lastaccess, string checksum)
{

    // Open connection to database
    SQLiteConnection sqliteCon = new SQLiteConnection(@"Data Source=" + connstring);
    sqliteCon.Open();
    using (SQLiteTransaction SQLiteTrans = sqliteCon.BeginTransaction())
    {
        using (SQLiteCommand cmd = sqliteCon.CreateCommand())
        {
            // Insert a new file record
            cmd.CommandText = "INSERT INTO folders(dirname, creationtime, lastwrite, lastaccess, checksum)" + " VALUES (?,?,?,?,?)";

            SQLiteParameter Field1 = cmd.CreateParameter();
            SQLiteParameter Field2 = cmd.CreateParameter();
            SQLiteParameter Field3 = cmd.CreateParameter();
            SQLiteParameter Field4 = cmd.CreateParameter();
            SQLiteParameter Field5 = cmd.CreateParameter();

            cmd.Parameters.Add(Field1);
            cmd.Parameters.Add(Field2);
            cmd.Parameters.Add(Field3);
            cmd.Parameters.Add(Field4);
            cmd.Parameters.Add(Field5);
            Field1.Value = dirname;
            Field2.Value = creationtime;
            Field3.Value = lastwrite;
            Field4.Value = lastaccess;
            Field5.Value = checksum;

            cmd.ExecuteNonQuery();
        }

        SQLiteTrans.Commit();
    }
    sqliteCon.Dispose();
}
\$\endgroup\$
2
  • \$\begingroup\$ First suggestion is for public int fileExists you are returning an int that has meaning to it. It would be better to make a enum with those values so that there is a name attached to those numbers. This way in the future you'll know what those values returned mean without having to reread your code \$\endgroup\$ Apr 21, 2013 at 20:02
  • \$\begingroup\$ absolutely! and thanks. It truly is easier to work with, as you can see from my own comments trying to track what return did what. \$\endgroup\$
    – intaipei
    Apr 24, 2013 at 10:22

1 Answer 1

1
\$\begingroup\$

Just a few comments:

  1. Try to use the 'using' construct for SQLiteTransaction, SQLiteCommand and SQLConnection objects everywhere, not just in selected places. Get rid of explicit calls to Dispose() method. Also there is a connection string parameter 'Pooling=true', which tells driver to return previously opened connection to the connection pool and reuse it later. This will improve performance.
  2. Be consistent with naming of your variables. Try to use camelCase for every variable name. There are several places in your code where you start using all lowercase for no good reason.
  3. Try to use parameterization in all queries. The 'SELECT' query concatenates filename directly, but should use parameter instead.
  4. Try to avoid plain numeric return values. You should use enum with self-descriptive member names instead. This will also make the comment describing return codes redundant. Also the function 'fileExists' does a little bit more than just checking if the file exists. You should probably refactor it into several functions with distinct responsibility and self-documenting names.
\$\endgroup\$
3
  • \$\begingroup\$ Thank you nullop! I would assume that I should still be closing the connection if I set pooling to true? \$\endgroup\$
    – intaipei
    Apr 24, 2013 at 10:14
  • \$\begingroup\$ Thank you for your time nullop! Point well taken on enum! I have looked into it and have added it to my code. I am curious, what is the detriment to a direct concat in the select string? Is it just bad SOP or is there something else to learn here? \$\endgroup\$
    – intaipei
    Apr 24, 2013 at 10:22
  • \$\begingroup\$ If you are using 'using' construct then Close() is called for you automatically by Dispose() method. If you're not using the dispose pattern, then you should call Close() explicitly to let the driver return connection back to the pool. As for the concatenation, it could possibly lead to sql injection if the part of filename comes from user input and is inserted into the sql query text. Parameters are processed automatically to avoid possible sql injections. Also parameterization allows sql engine to re-use previously calculated execution plan and reduces the load applied to sql engine. \$\endgroup\$
    – nullop
    Apr 25, 2013 at 10:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.