Simple class to access MySQL database

Question

I have written this simple DBHelper class to access a MySQL database. I am rethrowing a more generic exception so that the client side can take some action like showing a friendly message on the screen. Is it ok, or can something else be done?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

using MySql.Data.MySqlClient;

namespace DBLibrary
{
    public class DBHelper
    {
        private MySqlConnection _connection;
        private string _server;
        private string _database;
        private string _uid;
        private string _password;


        private MySqlCommand _command;

        public DBHelper(string server, string database, string uid, string password)
        {
            _connection = new MySqlConnection();

            _server = server;
            _database = database;
            _uid = uid;
            _password = password;

            _connection.ConnectionString = String.Format("server={0};database={1};uid={2};password={3}", _server, _database, _uid, _password);
        }

        public MySqlParameterCollection Parameters
        {
            get { return _command.Parameters; }
            set
            {
                foreach (MySqlParameter param in value)
                    _command.Parameters.Add(param);
            }
        }

        public bool OpenConnection()
        {
            try
            {
                _connection.Open();
                _command = new MySqlCommand() { Connection = _connection };
            }
            catch (MySqlException ex)
            {
                //When handling errors, you can your application's response based on the error number.
                //The two most common error numbers when connecting are as follows:
                //0: Cannot connect to server.
                //1045: Invalid user name and/or password.
                switch (ex.Number)
                {
                    case 0:
                        throw new Exception("Cannot connect to server. Contact administrator !");

                    case 1045:
                        throw new Exception("Invalid username/password. Please check your connection string and try again !");
                }
                return false;
            }
            return true;
        }

        private bool CloseConnection()
        {
            try
            {
                _connection.Close();
                _refreshCommand(null);
                return true;
            }
            catch (MySqlException ex)
            {
                if (ex.InnerException == null)
                    throw new Exception(ex.Message);
                throw new Exception(ex.InnerException.Message);
            }
        }

        public int ExecuteStoredProcedure(string name)
        {
            _command.CommandType = System.Data.CommandType.StoredProcedure;
            _command.CommandText = name;
            int count = _command.ExecuteNonQuery();

            _refreshCommand(_connection);
            return count;
        }

        private void _refreshCommand(MySqlConnection con)
        {
            _command = new MySqlCommand() { Connection = con };
        }        
    }
}

Answer 1

Property Parameters

• The setter adds the new parameters to the list - I would expect, that the list will be cleared before.
• The setter and getter throw a null reference exception if the connection was not opened before

Method OpenConnection

• If an expected exception occurs, an exception is thrown; otherwise the method returns false. That is IMHO unusually... I would prefer to return a result like (enum OpenConnectionResult { ServerNotAvailable, AuthorizationFailed, UnknownError, Succeess }).
• The error text of the exception can not be localized (that problem is also solved by the solution above)
• If throwing a new exception, it makes sense to pass the underlying exception as inner exception to the new one (new Exception("Cannot connect to server. Contact administrator !", ex);)

Method CloseConnection

• The methods returns always true or throws an exception.
• The method is private and will not be called within the class.

if (ex.InnerException == null)
    throw new Exception(ex.Message);
throw new Exception(ex.InnerException.Message);

1. Don't do that... You hide valuable information that may be useful for error analysis! Just throw the exception here... You can handle that kind of stuff from outside if required.

2. What if the inner exception has another inner exception? If you want to get the most underlying exception use something like while(ex.InnerException != null) { ex = ex.InnerException; }

Method ExecuteStoredProcedure

• The method throws a null reference exception if OpenConnection was not called.

Method _refreshCommand

• Methods start with a capital letter in c# (even private ones)
• Remember to dispose the command object!

---

General Design

Your class has some surprises for the user. The parameters can be set from outside, but after running a procedure, a new command is initialized and the modified parameter list is reset - very confusing! Even the error handling is not very usual.

I suppose, that you set the parameters before you are running ExecuteStoredProcedure!? If so, why not passing the command parameters to that method? There is no need for the class to know the command parameters.

IMHO that would simplify the usage of the class:

public class DBHelper
{
    private readonly MySqlConnection _connection;
    private readonly string _server;
    private readonly string _database;
    private readonly string _uid;
    private readonly string _password;

    public DBHelper(string server, string database, string uid, string password)
    {
        _server = server;
        _database = database;
        _uid = uid;
        _password = password;

        _connection = new MySqlConnection();
        _connection.ConnectionString = String.Format("server={0};database={1};uid={2};password={3}", _server, _database, _uid, _password);
    }

    public OpenConnectionResult OpenConnection()
    {
        try
        {
            _connection.Open();
        }
        catch (MySqlException ex)
        {
            switch (ex.Number)
            {
                case 0: return OpenConnectionResult.ServerNotAvailable;
                case 1045: return OpenConnectionResult.AuthorizationFailed;
                default: return OpenConnectionResult.UnknownError;
            }
        }
        catch (Exception ex)
        {
            return OpenConnectionResult.UnknownError;
        }
        return OpenConnectionResult.Succeess;
    }

    public void CloseConnection()
    {
        _connection.Close();
    }

    public int ExecuteStoredProcedure(string name, params MySqlParameter[] commandParamters)
    {
        using (var cmd = new new MySqlCommand())
        {
            cmd.CommandType = System.Data.CommandType.StoredProcedure;
            cmd.Connection = _connection;
            cmd.CommandText = name;
            cmd.Parameters.AddRange(commandParamters);
            int count = cmd.ExecuteNonQuery();
        }
        return count;
    }
}

Answer 2

• You certainly don't want another helper class. Helpers should be avoided. I'm sure you can think of a better name like DatabaseConnection
• You want you connection string to be valid so use the ConnectionStringBuilder that takes care of value validation.
• Now that it's a connection the method to open it can just be called Open and the for closing it just Close is ok.
• You want to use it easily and close it automatically when you're done so make the class IDisposable and handle closing on dispose.
• The Open method should either return a bool or throw but not both as a matter of fact it should not return anything but just throw when something bad happens and the client should handle the exception. You don't want to return custom error codes. This is an old strategy and no longer valid in C#. The right way to do it is to throw. You can make it easier for the handler to get the right message by wrapping the exceptions with a custom exception.
• Don't bother the user with mysql-stuff like the parameters. He doesn't need to know what he's using. Let him use something he's familiar with like a dictionary.

Example:

public class DatabaseConnection : IDisposable
{
    private MySqlConnection _connection;

    public DatabaseConnection(string server, string database, string uid, string password)
    {
        _connection = new MySqlConnection();
        _connection.ConnectionString = new SqlConnectionStringBuilder()
        {
            DataSource = server,
            InitialCatalog = database,
            UserID = uid,
            Password = password
        };
    }

    public void Open()
    {
        try
        {
            _connection.Open();
        }
        catch (MySqlException ex)
        {
            throw new DatabaseConnectionException("Cannot connect to server.", ex);
        }
    }

    public void Close()
    {
        try
        {
            // if(_connection..) // check if opened and close..
            _connection.Close();
        }
        catch (MySqlException ex)
        {
            throw new DatabaseConnectionException("Cannot disconnect from server.", ex);
        }
    }

    public void Dispose()
    {
        Close();
    }

    public int ExecuteStoredProcedure(string name, Dictionary<string, object> parameters)
    {
        using (var cmd = new MySqlCommand() { Connection = _connection })
        {
            _command.CommandText = name;
            _command.CommandType = System.Data.CommandType.StoredProcedure;         

            foreach (var item in parameters)
            {
                _command.Parameters.AddWithValue(item.Key, item.Value);
            }
            var count = _command.ExecuteNonQuery();
            return count;
        }
    }

    public int ExecuteStoredProcedure(string name)
    {
        return ExecuteStoredProcedure(name, new Dictionary<string, object>());
    }
}

The custome exception:

public class DatabaseConnectionException : Exception
{
    internal DatabaseConnectionException(string message, MySqlException inner) : base(message, inner)
    {
    }

    public int ErrorCode => ((MySqlException)InnerException).ErrorCode;
}

Now you can easily put it in a using statement:

using (var conn = new DatabaseConnection()) 
{        
    conn.Open();
    conn.ExecuteStoredProcedure("abc");
}