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

Simple class to access MySQL database

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

Problem

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 Excep

Solution

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);


-
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.

-
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;
    }
}

Code Snippets

if (ex.InnerException == null)
    throw new Exception(ex.Message);
throw new Exception(ex.InnerException.Message);
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;
    }
}

Context

StackExchange Code Review Q#153545, answer score: 4

Revisions (0)

No revisions yet.