Accessing MySQL to make queries and scalars

Question

I've developed my own class in ASP.NET project to access MySQL and make queries and scalars, and read the results from it.

I want you to review my class and tell me where I've made a mistake.

Here are some questions that are important for me:

• Do I use a reference correctly for the connector class of MySQL in methods?
• Is it correct to make it NON-static class as it is? For the GC, memory reasons?
• Do I need to add some events, delegate for the autoclose for the MySQL connection, or is there no need in it, because out of scope it will be auto freed from memory?

public class MySQLGear
{
    private const string connStr = "Server = localhost; Database = self; Uid = someuser; Pwd = 1234;";

    public MySqlConnection CreateConnection()
    {
        MySqlConnection connMysql = new MySqlConnection(connStr);
        connMysql.Open();

        if (connMysql.State == ConnectionState.Open)
        {
            return connMysql;
        }
        else return null;
    }

    public MySqlDataReader GetReader(ref MySqlConnection connMysql, string queryMysql)
    {
        MySqlCommand cmdMysql = new MySqlCommand(queryMysql, connMysql);
        MySqlDataReader readerMysql = cmdMysql.ExecuteReader();
        return readerMysql;
    }

    public int MakeQuery(ref MySqlConnection connMysql, string queryMysql)
    {
        MySqlCommand cmdMysql = new MySqlCommand(queryMysql, connMysql);
        int result = cmdMysql.ExecuteNonQuery();
        return result;
    }

    public object MakeScalar(ref MySqlConnection connMysql, string queryMysql)
    {
        MySqlCommand cmdMysql = new MySqlCommand(queryMysql, connMysql);
        object result = cmdMysql.ExecuteScalar();
        return result;
    }
}

Answer 1

Question #1

No. You do not need to ref the connection object in your methods' parameters - think object oriented way not class oriented.

Question #2

The only way is to make this as a non static class. Avoid always statics when the current problem contains some kind of state management like a MySQL connection.

Question #3

No you don't but you should implement the IDisposable interface and apply the correct disposable pattern.

public class MySQLGear : IDisposable
{
    private readonly MySqlConnection _connection;

    public MySQLGear(string connectionString)
    {
        _connection = new MySqlConnection(connectionString);
    }

    protected MySqlConnection Connection()
    {
        if (_connection == null)
        {
            throw new ObjectDisposedException("The underlying connection has been closed");
        }

        if (_connection.State != ConnectionState.Open)
        {
            _connection.Open();
        }

        return _connection;
    }

    public MySqlDataReader GetReader(string queryMysql)
    {
        using (var cmdMysql = new MySqlCommand(queryMysql, Connection()))
        {
            return cmdMysql.ExecuteReader();
        }
    }

    public int MakeQuery(string queryMysql)
    {
        using (var cmdMysql = new MySqlCommand(queryMysql, Connection()))
        {
            return cmdMysql.ExecuteNonQuery();
        }
    }

    public object MakeScalar(string queryMysql)
    {
        using (var cmdMysql = new MySqlCommand(queryMysql, Connection()))
        {
            return cmdMysql.ExecuteScalar();
        }
    }

    ~MySQLGear()
    {
        Dispose(false);
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (_connection != null)
        {
            if (_connection.State != ConnectionState.Closed) 
            {
                _connection.Dispose();
            }
            _connection = null;
        }
    }
}

Answer 2

It's a bad practice to keep an open connection. I'd advice you to store a connection string and open connection each time within a using block.

Something like this (just change to mysql funcs):

public class OleDbGear
{
    private readonly string _connectionString;

    /// <summary>
    ///     Default constructor.
    /// </summary>
    /// <param name="connString">Connection string.</param>
    public OleDbGear(string connSrting)
    {
        _connectionString = connString;
    }

    /// <summary>
    ///     Returns open connection.
    /// </summary>
    private OleDbConnection GetOpenConnection()
    {
        var dbConnection = new OleDbConnection(_connectionString);
        dbConnection.Open();
        return dbConnection;
    }

    private T Execute <T>(string query, Action<OleDbCommand> paramsSetup, Func<OleDbCommand, T> func)
    {
        using (OleDbConnection connection = GetOpenConnection())
        using (var cmd = new OleDbCommand(query, connection))
        {
            if (paramsSetup != null)
                paramsSetup(cmd);
            return func(cmd);
        }
    }

    public int ExecuteNonQuery(string query, Action<OleDbCommand> paramsSetup = null)
    {
        return Execute(query, paramsSetup, cmd => cmd.ExecuteNonQuery());
    }

    public object ExecuteSclar(string query, Action<OleDbCommand> paramsSetup = null)
    {
        return Execute(query, paramsSetup, cmd => cmd.ExecuteScalar());
    }

    public T ExecuteReader <T>(string query, Func<OleDbDataReader, T> converter, Action<OleDbCommand> paramsSetup = null)
    {
        return Execute(query, paramsSetup, cmd =>
            {
                using (OleDbDataReader reader = cmd.ExecuteReader())
                {
                    if (reader == null)
                        throw new Exception("DB error");
                    return converter(reader);
                }
            });
    }
}

Wrapping all in using blocks ensures correct disposing of all resources. Action<OleDbCommand> paramsSetup helps you to provide parameters of your query (no injection). Func<OleDbDataReader, T> converter just converts reader object into usable result.

You also could consider creating a sup method:

    public IEnumerable<T> ExecuteAndReadToEnd <T>(string query, Func<OleDbDataReader, T> lineReader, Action<OleDbCommand> paramsSetup = null)
    {
        return ExecuteReader(query, reader =>
            {
                var results = new List<T>();
                while (reader.Read())
                    results.Add(lineReader(reader));
                return results;
            }, 
            paramsSetup);
    }

as in most cases you are going to read all lines.

Usage example (suppose Instance is your reference to a db):

...
    public IEnumerable<string> GetAllPurchases(int customerId)
    {
        var query = "SELECT Purchase FROM Purchases WHERE CustomerId=@id";
        return Instance.ExecuteAndReadToEnd(query, reader => reader.GetString(0),
                                            cmd => cmd.Parameters.AddWithValue("@id", customerId));
    }

Good luck!