Return to Revisions

public class DbErrorEventArg : EventArgs
{
 private String _ErrorMsg;
 public String ErrorMsg
 {
 set { this._ErrorMsg = value; }
 get { return this._ErrorMsg; }
 }
 private String _LastQuery;
 public String LastQuery
 {
 set { this._LastQuery = value; }
 get { return this._LastQuery; }
 }
} 

• If you don't need to have any validation in the setter of a property you should consider using auto-implemented properties.

• The idiomatic way would be to use the string alias instead of the String class.

• Why should a consumer of an event which passes this event arguments be able to change any of the properties ? Using a private setter in combination with a constructor which takes the values will make this better.

Implementing the mentioned points will lead to

public class DbErrorEventArg : EventArgs
{
 public string ErrorMsg { get; private set; }
 public string LastQuery { get; private set; }
 public DbErrorEventArg (string errorMesg, string lastQuery)
 {
 ErrorMsg = errorMsg;
 LastQuery = lastQuery;
 }
}

MssqlDb

private SqlConnection MySqlConnection; 

this is just misleading. Having a class named MssqlDb shouldn't have a field named MySqlConnection. Naming the field just connection would be much better. While talking about fields, fields (variables) should be named using camelCase casing. See NET naming guidelines

public MssqlDb(SqlConnection P_SqlConnection, Int32 P_Timeout = 120)
{
 MySqlConnection = P_SqlConnection;
 if (null == MySqlConnection)
 throw new Exception("Invalid MSSQL Database Conection Handle");
 if (MySqlConnection.State != System.Data.ConnectionState.Open)
 throw new Exception("MSSQL Database Connection Is Not Open");
 MyTimeout = P_Timeout;
} 

Due to the lack of vertical space (new lines) which could group related code, this is hard to read.

Regarding the constructor arguments, please see the naming guideline.

Not using braces {} although they might be optional can lead to serious bugs and makes your code error prone. The decision if you use braces or don't is yours to decide, but if you choose one style you should stick to it. Right now you are using both styles.

internal void LogDbException(String ErrorMsg)
{
 if (OnError != null)
 {
 OnError(this, new DbErrorEventArg()
 {
 ErrorMsg = ErrorMsg,
 LastQuery = LastQuery
 });
 }
} 

A very misleading method name. If I read this name I will suppose that there is some logging taken place. To fix any threading issues the OnError delegate should be assigned to a local variable like so

internal void LogDbException(String ErrorMsg)
{
 var errorDelegate = OnError;
 if (errorDelegate != null)
 {
 errorDelegate (this, new DbErrorEventArg()
 {
 ErrorMsg = ErrorMsg,
 LastQuery = LastQuery
 });
 }
} 

private T Retry<T>(Func<T> MyFunc)
{
 try
 {
 int RetryCount = 30;
 TimeSpan RetryDelay = TimeSpan.FromSeconds(1);
 while (true)
 {
 try
 {
 if (MySqlConnection.State == System.Data.ConnectionState.Closed)
 {
 MySqlConnection.Open();
 }
 return MyFunc();
 }
 catch (SqlException e)
 {
 --RetryCount;
 if (RetryCount <= 0) { throw; }
 if (e.Number == 1205)
 {
 LogDbException("[ Attempts Left : " + RetryCount + " ] Failed to execute Query, SQL Deadlock occured. Retrying again in " + RetryDelay.Seconds + " seconds ...");
 }
 else if (e.Number == -2)
 {
 LogDbException("[ Attempts Left : " + RetryCount + " ] Failed to execute Query, SQL Timeout occured. Retrying again in " + RetryDelay.Seconds + " seconds ...");
 }
 else { throw; }
 Thread.Sleep(RetryDelay);
 }
 }
 }
 catch (Exception e)
 {
 LogDbException("Failed to execute SQL Query : " + e.Message);
 return default(T);
 }
}

I don't like this double try..catch style. You should try to refactor this.

The magic numbers 1205 and -2 should be extracted to meaningful constants. Do you still know what the numbers mean ? I don't think so.

It would be better to compose the error messages by using string.Format() instead of string concatenation. For the general exception, you should use the StackTrace value too.

public T ExecuteScalar(String MyQuery)

Instead of using Parse() in combination with try..catch you should use TryParse().

Having ToString() in a try..catch for typeof(string) isn't needed, because this won't throw (for 99.99999%).

This would lead to (also renaming the local fields)

public T ExecuteScalar<T>(String query)
{
 LastQuery = query;
 return Retry(() =>
 {
 using (SqlCommand sqlCommand = new SqlCommand(query, MySqlConnection))
 {
 sqlCommand.CommandTimeout = MyTimeout;
 object result = sqlCommand.ExecuteScalar();
 
 if (null == result || result is DBNull)
 {
 return default(T);
 }
 Type type = typeof(T);
 if (type == typeof(int))
 {
 int value;
 int.TryParse(result.ToString(), out value);
 return value;
 }
 if (type == typeof(decimal))
 {
 decimal value;
 decimal.TryParse(result.ToString(), out value);
 return value;
 }
 if (type == typeof(string))
 {
 return result.ToString();
 }
 return (T)result;
 }
 });
}

public List<Dictionary<String, Object>> ExecuteReader(String MyQuery)
{
 LastQuery = MyQuery;
 return Retry(() =>
 {
 using (SqlCommand MySqlCommand = new SqlCommand(MyQuery, MySqlConnection))
 {
 MySqlCommand.CommandTimeout = MyTimeout;
 List<Dictionary<string, object>> Results = new List<Dictionary<string, object>>();
 using (SqlDataReader MySqlDataReader = MySqlCommand.ExecuteReader())
 {
 if (null != MySqlDataReader)
 {
 if (MySqlDataReader.HasRows)
 {
 while (MySqlDataReader.Read())
 {
 Dictionary<string, object> ResultData = new Dictionary<string, object>();
 for (int i = 0; i < MySqlDataReader.FieldCount; i++)
 {
 string ColumnName = MySqlDataReader.GetName(i);
 object ColumnData = MySqlDataReader[ColumnName];
 ResultData.Add(ColumnName, ColumnData);
 }
 Results.Add(ResultData);
 }
 }
 }
 MySqlDataReader.Close();
 }
 return Results;
 }
 });
}

• you have some naming issues regarding the names and the casing
• by enclosing the SqlDataReader in an using block the reader will be automatically closed.
• instead of having such arrow code you should return early if possible.
• the Read() method of the datareader returns false if there aren't anymore rows to read, so it is superflous to check for HasRows

Implementing these changes lead to

public List<Dictionary<String, Object>> ExecuteReader(String query)
{
 LastQuery = query;
 return Retry(() =>
 {
 using (SqlCommand command = new SqlCommand(query, MySqlConnection))
 {
 command.CommandTimeout = MyTimeout;
 List<Dictionary<string, object>> results = new List<Dictionary<string, object>>();
 using (SqlDataReader reader = command.ExecuteReader())
 {
 if (null == reader) { return results; }
 
 while (reader.Read())
 {
 Dictionary<string, object> resultData = new Dictionary<string, object>();
 for (int i = 0; i < reader.FieldCount; i++)
 {
 string columnName = reader.GetName(i);
 object value = reader[columnName];
 resultData.Add(columnName, value);
 }
 results.Add(resultData);
 }
 }
 return results;
 }
 });
} 

The remaining code just needs some facelifting regarding method arguments and variable names.

Using regions can be considered as code smell. See: Are #regions an antipattern or code smell?