patterncsharpMinor
Simple class to access MySQL database
Viewed 0 times
simpledatabasemysqlclassaccess
Problem
I have written this simple
```
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
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
Method OpenConnection
Method CloseConnection
-
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
Method ExecuteStoredProcedure
Method _refreshCommand
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
IMHO that would simplify the usage of the class:
- 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.