Using SQLite in WPF Project

Question

I am new to WPF and SQLite. I want to use a SQLite database in my project for managing accounts.

I create a helper class for the SQLite connection and CRUD operations, which I initialize in my MainViewModel. Once initialized, the helper checks whether the database already exists; if not, it creates the database and sets a password.

Then, in my example below, it checks if a admin password is set and performs a login request.

In my MainViewModel I use it like this:

SQLiteHelper sqlite = new SQLiteHelper();

if (!sqlite.CheckAdmin())
{
[...]
}
User login = sqlite.Login(Loginname, HelperClass.MD5Hash(Password));

Here's the definition:

   class SQLiteHelper
    {
        private SQLiteConnection sqlConn;
        private static string sqliteFile = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData) + "/database.db";  // set folder for database
        private static string sqlitePw = "databasepw"; // set password for database

        public SQLiteHelper()
        {
            // check if database file exist when not create with password
            if (!File.Exists(sqliteFile))
            {
                sqlConn = new SQLiteConnection("Data Source=" + sqliteFile);
                sqlConn.SetPassword(sqlitePw);
            }
            sqlConn = new SQLiteConnection("Data Source=" + sqliteFile + ";Password=" + sqlitePw); // connect to database

            // create tables, when not exist
            string query =
                "CREATE TABLE IF NOT EXISTS user (id INTEGER PRIMARY KEY AUTOINCREMENT, 'login' TEXT, 'firstname' TEXT, 'lastname' TEXT, 'persid' TEXT, 'password' Text, 'role' INTEGER, 'first' INTEGER, 'active' INTEGER);" +
                "CREATE TABLE IF NOT EXISTS userRigths (id INTEGER PRIMARY KEY, 'lwe' INTEGER, 'lwa' INTEGER, 'lwb' INTEGER, 'lwi' INTEGER, 'mwe' INTEGER, 'mwa' INTEGER, 'mwb' INTEGER, 'mwi' INTEGER, 'vacc' INTEGER, 'vadr' INTEGER);" +
                "CREATE TABLE IF NOT EXISTS admin ('password' Text);";
            queryNon(query);
        }


        // check if admin password exist
        public bool CheckAdmin()
        {
            sqlConn.Open();
            var command = sqlConn.CreateCommand();
            command.CommandText = "SELECT * FROM admin";
            SQLiteDataReader reader = command.ExecuteReader();
            bool rows = reader.HasRows;
            sqlConn.Close();
            return rows;
        }

        // login user
        // if login doesnt match return an empty user
        public User Login(string login, string pw)
        {
            User user = new User();

            sqlConn.Open();

            if (!login.Equals("Admin"))
            {
                var command = sqlConn.CreateCommand();
                command.CommandText = "SELECT * FROM user LEFT JOIN userRigths ON user.id = userRigths.id WHERE user.login = '" + login + "' AND user.password = '" + pw + "'";
                SQLiteDataReader reader = command.ExecuteReader();
                if (reader.HasRows)
                {
                    while (reader.Read())
                    {
                        user.Id = reader.GetInt32(0);
                        user.Login = reader.GetString(1);
                        user.Role = reader.GetInt16(6);
                        break;
                    }
                }
            }
            else
            {
                var command = sqlConn.CreateCommand();
                command.CommandText = "SELECT * FROM admin WHERE admin.password = '" + pw + "'";
                SQLiteDataReader reader = command.ExecuteReader();
                if (reader.HasRows)
                {
                    user.Id = 0;
                    user.Login = "Admin";
                    user.Role = -1;
                }
            }

            sqlConn.Close();

            return user;
        }

        // methode for querys without response
        private void queryNon(string query)
        {
            sqlConn.Open();
            var command = sqlConn.CreateCommand();
            command.CommandText = query;
            command.ExecuteNonQuery();
            sqlConn.Close();
        }
    }

My code is based on some snippets which I found on the Web. I think it is very simple and certainly not optimal. What can I improve?

Answer 1

Refactoring for Modularity

The whole point of using classes is making your code more modular. The ViewModel doesn't care about the database type. It could be SQLite, but it could also be PostgreSQL, Microsoft Sql Express or just a plain old file. Consider the following:

ApplicationDatabase database = new ApplicationDatabase();

if (!database.CheckAdmin())
{
[...]
}
User login = database.Login(Loginname, HelperClass.MD5Hash(Password));

If you choose to use a different database system later on, you'll only have to change your ApplicationDatabase code - the ViewModel code will still be valid.

Use a Fragmented Model

Your ViewModel wants to consume model objects and translate them to view objects. Don't make it access the database directly!

Instead, add an ApplicationData class to your model. The ApplicationData should interact with the ApplicationDatabase to get the data it needs, store it in objects, and let the ViewModel access those.

Refactoring for Readability

• Do one thing

  "Functions should do one thing. They should do it well. They should do it only." Robert C. Martin, Clean Code

  Your functions do more than one thing. This makes your code less readable, as each part must be understood seperately. Split them up!

  public ApplicationDatabase()
  {
      if (!DatabaseExists())
      {
          CreateDatabase(databaseFile, databasePassword);
          CreateTables();
      }
      ConnectToDatabase();
  }
  
  public User Login(string login, string password)
  {
      if (!login.Equals("Admin"))
      {
          user = GetUser(login, password);
      }
      else
      {
          user = GetAdmin(login, password);
      }
      return user;
  }
  

• Use descriptive names.

  Don't write a comment before each function to describe what it does. If you use that function in your ViewModel later on, you won't have those comments! Wouldn't it be better if the functions name gave you all the information you need?

  public bool AdminPasswordExists(){}
  private void QueryWithoutResponse(string query){}
  

  Even if it is a bit long, you have autocompletion in every decent IDE to help you type faster.

Answer 2

SQL Injection

I'm outside my known languages here, but this statement rings the alarm bells:

command.CommandText = "SELECT * FROM user LEFT JOIN userRigths ON user.id = userRigths.id WHERE user.login = '"
    + login + "' AND user.password = '" + pw + "'";

Ignoring the odd spelling (possible typo - I'd write userRights), this is unsafe because login and pw can contain anything. What happens if one of your users is Little Bobby Tables?

What you need to do is write a string with parameters, and then provide the values as arguments when you execute the query. It might look something like:

command.CommandText = "SELECT * FROM user LEFT JOIN userRigths ON user.id = userRigths.id"
                    + "WHERE user.login = @login AND user.password = @password";
command.Parameters.AddWithValue("@login", login);
command.Parameters.AddWithValue("@password", pw);
SQLiteDataReader reader = command.ExecuteReader();

The code above is adapted from bobby-tables.com and a Stack Overflow question.