Restoring data from database to list with SQLite

Question

I have a Winforms project in which I have a screen which loads a questionnaire. It restores answers the user has previously entered by restoring the saved results from the database to a single list (see code below).

The answers the user fills in are saved in my SQLite database table 'Results' with a result_id, assessment_id (the assessment this result belong to), question_id (the question this result belongs to), m_answer_id (the given maturity answer to the question), c_answer_id (the given compliance answer to the question), and notes (notes the user typed in for this question).

The assessment information such as the name, the standards, the questions to be asked etc. are stored in my 'CurrentAssessmentModel' class.

For an assessment, a user can specificy multiple question standards. For each question standard, both a maturity and a compliance standard are chosen. These standards create the questions and the answer possibilities (it is multiple choice).

The _databaseController methods simply open the connection to the database and executes the query with the arguments I provide. The QueryDatabase method has two parameters: one for the query, one for the database column I want to select from.

Does anyone know how I can improve this process (on the database/design/code side)? It is quite slow because I perform quite some queries.

The method in CurrentAssessmentModel that gets the results from the database

    // Returns the results that have been selected up to this point.
    public List<string> GetResults()
    {
        var results = new List<string>();

        try
        {
            var questions = new List<string>();

            for(int i = 0; i < questionsStandard.Count; i++)
            {
                var temp = _databaseController.QueryDatabase("SELECT * FROM Questions WHERE question_standard = '" +
                questionsStandard[i] + "'", "question_id");

                questions.AddRange(temp);
            }

            for (int i = 0; i < questions.Count; i++)
            {
                results.Add(_databaseController.QueryDatabase("SELECT * FROM Questions WHERE question_id = '" +
                    questions[i] + "'", "question_value")[0]);

                var tempMaturityAnswers = _databaseController.QueryDatabase("SELECT * FROM MaturityAnswers WHERE m_answer_id = " +
                    "(SELECT m_answer_id FROM Results WHERE assessment_id = '" + assessmentId + "' AND question_id = '" + questions[i] + "')",
                    "m_answer_value");
                if (tempMaturityAnswers != null)
                {
                    results.Add(tempMaturityAnswers[0]);
                }
                else results.Add("");

                var tempCompliacenAnswers = _databaseController.QueryDatabase("SELECT * FROM ComplianceAnswers WHERE c_answer_id = " +
                    "(SELECT c_answer_id FROM Results WHERE assessment_id = '" + assessmentId + "' AND question_id = '" + questions[i] + "')",
                    "c_answer_value");
                if (tempCompliacenAnswers != null)
                {
                    results.Add(tempCompliacenAnswers[0]);
                }
                else results.Add("");

                var tempNotes = _databaseController.QueryDatabase("SELECT * FROM Results WHERE assessment_id = '" + assessmentId + "' AND question_id = '" + questions[i] + "'",
                    "notes");
                if (tempNotes != null)
                {
                    results.Add(tempNotes[0]);
                }
                else results.Add("");
            }
        }
        catch (Exception)
        {
            throw;
        }

        return results;
    }

The CurrentAssessment's members and constructor

public class CurrentAssessmentModel: ICurrentAssessmentModel
{
    // Private member.
    private IDatabaseController _databaseController;

    // Private members that make up one assessment.
    private string assessmentName;
    private string assessmentId;
    private string assessorName;
    private string companyName;
    private List<string> questionsStandard;
    private List<string> mAnswersStandard;
    private List<string> cAnswersStandard;
    private string zone;

    private string maturityThreshold;
    private string complianceThreshold;

    // Public members.
    public List<List<string>> Questions { get; set; }
    public List<List<string>> MaturityAnswers { get; set; }
    public List<List<string>> ComplianceAnswers { get; set; }

    // Sets all assessment variables by inserting them into the database.
    public CurrentAssessmentModel(string name, IDatabaseController databaseController)
    {
        _databaseController = databaseController;

        try
        {
            assessmentName = name;
            assessmentId = _databaseController.QueryDatabase("SELECT * FROM Assessments WHERE assessment_name = '" + assessmentName + "'",
                "assessment_id")[0];
            assessorName = _databaseController.QueryDatabase("SELECT * FROM Assessments WHERE assessment_name = '" + assessmentName + "'",
                "assessor_name")[0];
            companyName = _databaseController.QueryDatabase("SELECT * FROM Assessments WHERE assessment_name = '" + assessmentName + "'",
                "company_name")[0];
            questionsStandard = new List<string>(_databaseController.QueryDatabase(
                "SELECT * FROM Assessments WHERE assessment_name = '" + assessmentName + "'", "questions_standard"));
            mAnswersStandard = new List<string>(_databaseController.QueryDatabase(
                "SELECT * FROM Assessments WHERE assessment_name = '" + assessmentName + "'", "m_answers_standard"));
            cAnswersStandard = new List<string>(_databaseController.QueryDatabase(
                "SELECT * FROM Assessments WHERE assessment_name = '" + assessmentName + "'", "c_answers_standard"));

            var zones = _databaseController.QueryDatabase(
                "SELECT * FROM Assessments WHERE assessment_name = '" + assessmentName + "'", "zone");
            if (zones != null && zones.Any())
            {
                zone = zones[0]; ;
            }

        }
        catch (Exception)
        {
            throw;
        }


    }
}

DatabaseController

public class DatabaseController: IDatabaseController
{
    private SQLiteConnection dbConnection;

    // Initializes the database controller with the connection string.
    public DatabaseController()
    {
        dbConnection = new SQLiteConnection("DATA SOURCE = stageDB01.sqlite; VERSION = 3;");
    }

    // Inserts value(s) into colum(s) of a certain table in the database.
    public void InsertToDatabase(string table, string columns, string values)
    {
        try
        {
            if (dbConnection.State != System.Data.ConnectionState.Open)
            {
                dbConnection.Open();
            }

            string insertionString = "INSERT INTO " + table + " (" + columns + ") VALUES " + values + "";
            SQLiteCommand insertionCommand = new SQLiteCommand(insertionString, dbConnection);
            insertionCommand.ExecuteNonQuery();
        }
        catch (Exception)
        {
            throw;
        }
        finally
        {
            if (dbConnection.State != System.Data.ConnectionState.Closed)
            {
                dbConnection.Close();
            }
        }

    }

    // Queries the database.
    public List<string> QueryDatabase(string query, string column)
    {
        List<string> queryResult = new List<string>();

        try
        {
            if (dbConnection.State != System.Data.ConnectionState.Open)
            {
                dbConnection.Open();
            }

            SQLiteCommand queryCommand = new SQLiteCommand(query, dbConnection);
            SQLiteDataReader queryReader = queryCommand.ExecuteReader();

            while (queryReader.Read())
            {
                if (queryReader[column].GetType().ToString() != "System.DBNull")
                {
                    queryResult.Add(queryReader[column].ToString());
                }
            }

            if (queryResult.Any())
            {
                return queryResult;
            }

            return null;
        } 
        catch(Exception)
        {
            throw;
        }
        finally
        {
            if (dbConnection.State != System.Data.ConnectionState.Closed)
            {
                dbConnection.Close();
            }
        }
    }

    // Updates an excisting database record.
    public void UpdateDatabase(string table, string column, string value)
    {
        try
        {
            if (dbConnection.State != System.Data.ConnectionState.Open)
            {
                dbConnection.Open();
            }

            string updateString = "UPDATE " + table + " SET " + column + " = " + value + "";
            SQLiteCommand updateCommand = new SQLiteCommand(updateString, dbConnection);
            updateCommand.ExecuteNonQuery();
        }
        catch (Exception)
        {
            throw;
        }
        finally
        {
            if (dbConnection.State != System.Data.ConnectionState.Closed)
            {
                dbConnection.Close();
            }
        }
    }

    // Deletes row from database.
    public void DeleteFromDatabase(string table, string condition)
    {
        try
        {
            if (dbConnection.State != System.Data.ConnectionState.Open)
            {
                dbConnection.Open();
            }

            string deletionString = "DELETE FROM " + table + " " + condition + "";
            SQLiteCommand deletionCommand = new SQLiteCommand(deletionString, dbConnection);
            deletionCommand.ExecuteNonQuery();
        }
        catch (Exception)
        {
            throw;
        }
        finally
        {
            if (dbConnection.State != System.Data.ConnectionState.Closed)
            {
                dbConnection.Close();
            }
        }
    }

}

Answer 1

Your question is missing two crucial parts: IDatabaseController and the database model so I can give you only general advice.

SQLiteDataReader

    var questions = new List<string>();

    for (int i = 0; i < questionsStandard.Count; i++)
    {
        var temp = _databaseController.QueryDatabase("SELECT * FROM Questions WHERE question_standard = '" + questionsStandard[i] + "'", "question_id");

        questions.AddRange(temp);
    }

You don't need to query each record separately to add it to a list. Use the SqliteDataReader to get all data records and filter them with linq if necessary.

For example:

using (SqliteCommand cmd = new SqliteCommand(query, connection))
using (var reader = cmd.ExecuteReader())
{
    while (reader.Read())
    {
        questions.Add(reader.GetString(0));
    }
}

This should be much faster.

SQLiteParameter as SQL Injection Protection

"SELECT * FROM Questions WHERE question_standard = '" + questionsStandard[i] + "'", "question_id"

You can simplify your queries by using the SQLiteParameter. By doing this you also protect your queries from SQL injection. So the above becomes:

command.CommandText = "SELECT * FROM Questions WHERE question_standard = @question_standard";
command.CommandType = CommandType.Text;
command.Parameters.Add(new SQLiteParameter("@question_standard", i));

---

Exceptions

catch (Exception)
{
    throw;
}

This has no use. You can as well remove the entire try/catch.

See this question on SO for more information how to correctly use the try/catch: The difference between try/catch/throw and try/catch(e)/throw e

Other issues

else results.Add("");

Is this really necessary? It looks like a bad design.

---

My gut tells me that at least some of the selects could be replaced by joins but without the database model it's impossible to write them.

DatabaseController

private SQLiteConnection dbConnection;

Usually it's not necessary to cache the connection. It should be disposed as soon as possible.

"DATA SOURCE = stageDB01.sqlite; VERSION = 3;"

Consider making the datasource file name a constant or a parameter to the constructor or possbily use the connectionStrings section of the app.config to configure it.

SQLiteCommand queryCommand = new SQLiteCommand(query, dbConnection);
SQLiteDataReader queryReader = queryCommand.ExecuteReader();

All SQLiteCommands and SQLiteDataReader need to be disposed. Wrap them in usings.

public void UpdateDatabase(string table, string column, string value)

Consider returning an int here for the number of affected rows. This can be quite useful.

    catch (Exception)
    {
        throw;
    }

Again, this has absolutely no use whatsoever ;-) You can leave it out and nothing will change.

SQL Injection protection Level 2

string insertionString = "INSERT INTO " + table + " (" + columns + ") VALUES " + values + "";

Apart from using parameters you can also sanitize table names by using the following pattern:

var dbProviderFactory = DbProviderFactories.GetFactory(connection);
using (var commandBuilder = dbProviderFactory.CreateCommandBuilder())
{
    var quote = new Func<string, string>(identifier => commandBuilder.QuoteIdentifier(identifier));

    var sanitizedTableName = quote(tableName);
}

Return values for collections

public List<string> QueryDatabase(string query, string column)
return null;

This is not a good practice. APIs returning collections should return empty collections rather then null. It'll save you the trouble of checking for null everywhere so you can just write someCollection.Any() instead of someCollection != null && someCollection.Any().