HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

MySQL database library with multiple connections

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withdatabasemysqlmultiplelibraryconnections

Problem

I am writing this question to get some advice on improving this Database Management library. I'll explain a little about it:

Database Manager - DatabaseManager is the holder, it generates new connections when the NewConnection is called, it returns a new DatabaseConnection with the saved connection string.

Database Connection - DatabaseConnection is a connection containing a new connection that's created on each call from DatabaseManager.

Usage:

using (var databaseConnection = Serber.GetDatabase().NewDatabaseConnection)
{
    databaseConnection.SetQuery("SELECT * FROM `table` WHERE `enabled` = '1' ORDER BY `name` DESC;");
    databaseConnection.Open();

    using (MySqlDataReader Reader = databaseConnection.ExecuteReader())
    {
        while (Reader.Read())
        {
            try
            {
                // do some work
            }
            catch (DatabaseException ex)
            {
                log.Error("Unable to load item for ID [" + Reader.GetInt32("id") + "]", ex);
            }
        }
    }
}


DatabaseManager:

```
internal sealed class DatabaseManager
{
private readonly string _connectionString;

public DatabaseManager()
{
var connectionString = new MySqlConnectionStringBuilder
{
ConnectionLifeTime = (60 * 5),
ConnectionTimeout = 30,
Database = Hariak.HariakServer.Config.GetConfigValueByKey("database.mysql.database"),
DefaultCommandTimeout = 120,
Logging = false,
MaximumPoolSize = uint.Parse(Hariak.HariakServer.Config.GetConfigValueByKey("database.mysql.pool_maxsize")),
MinimumPoolSize = uint.Parse(Hariak.HariakServer.Config.GetConfigValueByKey("database.mysql.pool_minsize")),
Password = Hariak.HariakServer.Config.GetConfigValueByKey("database.mysql.password"),
Pooling = Hariak.HariakServer.Config.GetConfigValueByKey("database.mysql.pooling") == "1",
Port = uint.Parse(Ha

Solution

Your code looks great to me at first glance. I have some tips for you that could increase the performance of your database manager:

-
Go async

  • I wouldn't say that the official ADO.NET MySQL connector is bad. It does pretty well what it has to, but let's admit it, async programming is not that rare today. You should take a look at one particular connector repo on Github, which is a fresh, clean and fully async ADO.NET MySQL connector that also supports .NET Core.



-
Move the parameter and the command holder outside your DatabaseConnection class. You might not need a holder for parameters during every query. If you move those back to your connector (DatabaseManager) and just grab one when needed, you can spare some allocated space.

Like this:

DatabaseManager:

public MySqlConnection CreateConnectionObject() => Activator.CreateInstance(typeof(MySqlConnection)) as MySqlConnection;
public MySqlCommand CreateCommandObject() => Activator.CreateInstance(typeof(MySqlCommand)) as MySqlCommand;
public MySqlParameter CreateParameterObject() => Activator.CreateInstance(typeof(MySqlParameter)) as MySqlParameter;


-
Refactor

  • Only make methods and variables public, if they really must be exposed to the environment. Any internal method that shouldn't be called from the outside has to be private or protected.



  • It's better, if you have only one DatabaseManager instance (which is the connector) and all the other are DatabaseConnection instances that link back to the DatabaseManager (which are the individual database interfaces and should only one per database exist). First you should create your DatabaseManager connector, then you create one DatabaseConnection for each database and if you would like to commit a query on that particular database, you can instruct the appropriate DatabaseConnection to do it.



  • Break your code into sections depending on the MySQL commands. Instead of one global query handler, you would have different methods for different actions (Select, Insert, Delete etc.). It results in a much cleaner code. Also it's great, if you have one MySqlCommand builder method, that decides whether parameters are needed or not.



See example below for what I mean:

private MySqlCommand CreateSqlCommand(MySqlConnection Connection, string Sql, params object[] Args)
{
    MySqlCommand SqlCommand = connector.CreateCommandObject(); // connector = DatabaseManager instance

    SqlCommand.Connection = Connection;
    SqlCommand.CommandText = Sql;
    SqlCommand.CommandTimeout = 300;

    if (Args.Length > 0)
    {
        MySqlParameter[] Params = new MySqlParameter[Args.Length];
        for (var i = 0; i  SelectAsync(string Sql, params object[] Args)
{
    try
    {
        using (MySqlCommand Command = CreateSqlCommand(CreateConnection(), Sql, Args))
            return await Command.ExecuteReaderAsync();
    }
    catch (Exception ex)
    {
        Console.WriteLine(ex.ToString());
        return null;
    }
}


-
Coding style

  • If an if, using, while or for statement is followed by a single action, then scoping ({ and }) is not needed.



  • Commenting is useful, when not overused. Personally, I dislike commenting everything, it makes my code messy and difficult to work with. If you name your variables after their purpose (as you did), comments are not that necessary, since the code speaks for itself.

Code Snippets

public MySqlConnection CreateConnectionObject() => Activator.CreateInstance(typeof(MySqlConnection)) as MySqlConnection;
public MySqlCommand CreateCommandObject() => Activator.CreateInstance(typeof(MySqlCommand)) as MySqlCommand;
public MySqlParameter CreateParameterObject() => Activator.CreateInstance(typeof(MySqlParameter)) as MySqlParameter;
private MySqlCommand CreateSqlCommand(MySqlConnection Connection, string Sql, params object[] Args)
{
    MySqlCommand SqlCommand = connector.CreateCommandObject(); // connector = DatabaseManager instance

    SqlCommand.Connection = Connection;
    SqlCommand.CommandText = Sql;
    SqlCommand.CommandTimeout = 300;

    if (Args.Length > 0)
    {
        MySqlParameter[] Params = new MySqlParameter[Args.Length];
        for (var i = 0; i < Args.Length; i++)
        {
            MySqlParameter Param = connector.CreateParameterObject(); // connector = DatabaseManager instance
            Param.ParameterName = "";
            Param.Value = Args[i];
            Params[i] = Param;
        }

        SqlCommand.Parameters.AddRange(Params);
    }
    return SqlCommand;
}

public async Task<MySqlDataReader> SelectAsync(string Sql, params object[] Args)
{
    try
    {
        using (MySqlCommand Command = CreateSqlCommand(CreateConnection(), Sql, Args))
            return await Command.ExecuteReaderAsync();
    }
    catch (Exception ex)
    {
        Console.WriteLine(ex.ToString());
        return null;
    }
}

Context

StackExchange Code Review Q#158131, answer score: 4

Revisions (0)

No revisions yet.