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

MySQL Database Library

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

Problem

so I created a database manager and wanted to know if I could improove it in any way? Thanks.

DatabaseManager.cs:

```
using MySql.Data.MySqlClient;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Sahara.Core.Database
{
class DatabaseManager
{
private readonly DatabaseSettings databaseSettings;
private readonly string connectionString;

public DatabaseManager(DatabaseSettings databaseSettings)
{
this.databaseSettings = databaseSettings;

MySqlConnectionStringBuilder mysqlConnectionString = new MySqlConnectionStringBuilder
{
ConnectionLifeTime = (60 * 5),
ConnectionTimeout = 30,
Database = databaseSettings.DatabaseName,
DefaultCommandTimeout = 120,
Logging = false,
MaximumPoolSize = databaseSettings.MaximumConnections,
MinimumPoolSize = 3,
Password = databaseSettings.DatabasePassword,
Pooling = true,
Port = 3306,
Server = databaseSettings.DatabaseHost,
UseCompression = false,
UserID = databaseSettings.DatabaseUsername
};

this.connectionString = mysqlConnectionString.ToString();
}

public bool ValidConnection()
{
try
{
using (var databaseConnection = GetDatabaseConnection())
{
databaseConnection.OpenConnection();
databaseConnection.SetQuery("SELECT 1+1;");
databaseConnection.RunQuery();
}
}
catch (MySqlException)
{
return false;
}

return true;
}

public DatabaseConnection GetDatabaseConnection()
{
return new DatabaseConnec

Solution

DatabaseManager

  • IMHO it is not a good idea to store the databaseSettings as instance variable because the object may be modified from outside. If so, the created connection string and the properties of databaseSettings are different.



DatabaseConnection

  • Use the method IsConnectionOpen within OpenConnection (instead of duplicating the comparisation)



-
It is possible to simplyfy Get* methods by return the result directly:

public string getString()
{
    try
    {
        object obj2 = mysqlCommand.ExecuteScalar();
        return obj2 != null ? obj2.ToString() : string.Empty;
    }
    catch (Exception exception)
    {
        Sahara.GetServer().GetLogManager().Log("MySQL Error: " + exception.Message, LogType.Error);
    }
}


-
I think it makes sense to dispose the connection, even if the state is not open. Check if the variable is not null instead to allow multiple calls of Dispose().

  • The get methods should renamed to Get



  • Finish should be renamed to finish



API

The API is a little bit confusing, because adding a query clears the list of parameters that was added before. That is something non-intuitive behavior that you have to know ;)

If you want to create something around the regular DB-API, I would prefer to create a query object that contains the SQL and all parameters and than execute that query object in one single operation so that the connection object doesn't has a "query state".

Abstraction

It is possible to abstract all that stuff so that the DatabaseManager and the DatabaseConection may work with any database management systems (not just MySQL).

If you want to support all DBMSs, just use the abstract types (DbConnection instead of MySqlConnection, DbCommand instead of MySqlCommand, DbParameter instead of MySqlParameter, and so on).

Code Snippets

public string getString()
{
    try
    {
        object obj2 = mysqlCommand.ExecuteScalar();
        return obj2 != null ? obj2.ToString() : string.Empty;
    }
    catch (Exception exception)
    {
        Sahara.GetServer().GetLogManager().Log("MySQL Error: " + exception.Message, LogType.Error);
    }
}

Context

StackExchange Code Review Q#144057, answer score: 3

Revisions (0)

No revisions yet.