I have a wrapper class for executing database queries from an ASP.NET application. Please provide suggestions on how I can improve this class. It is working fine. Anyone can use this class in their ASP.NET application.
using System;
using System.Collections;
using System.Configuration;
using System.Data;
using System.Data.SqlClient;
public class SPWrapper
{
private string _ConnectionString = string.Empty;
private Hashtable _ParamHT = new Hashtable();
private QueryMode _QMode = QueryMode.ExecuteNonQuery;
private string _SPName = string.Empty;
private SqlCommand _SQLCmd;
private string outputParams;
private const string prfOut = "#";
private SqlDataReader SPDataReader;
private SqlParameter SPParam;
private SqlTransaction SPTransaction;
public SPWrapper()
{
this._ConnectionString = ConfigurationManager.ConnectionStrings["MS_conn_string"].ToString();
}
public bool DBConnection()
{
try
{
SqlConnection connection = new SqlConnection(this._ConnectionString);
return true;
}
catch (Exception exception)
{
ErrorLog.AddException(exception);
return false;
}
}
public object ExecuteSP()
{
object obj3;
SqlConnection connection = new SqlConnection(this._ConnectionString);
try
{
SqlDataAdapter adapter;
this._SQLCmd = new SqlCommand(this._SPName, connection);
this._SQLCmd.CommandType = CommandType.StoredProcedure;
string pName = string.Empty;
bool flag = false;
if ((this._ParamHT != null) && (this._ParamHT.Count != 0))
{
Hashtable pDataType = (Hashtable) this.GetPDataType();
foreach (object obj2 in this._ParamHT.Keys)
{
pName = obj2.ToString();
if (pName.Substring(0, 1) == "#")
{
flag = true;
pName = pName.Replace("#", "");
}
pName = "@" + pName;
this.SetParam(pName, pDataType[pName].ToString(), this._ParamHT[obj2].ToString(), flag);
flag = false;
}
}
this._SQLCmd.Connection.Open();
DataSet dataSet = new DataSet();
if (this.Mode == QueryMode.ExecuteNonQuery)
{
this._SQLCmd.ExecuteNonQuery();
adapter = new SqlDataAdapter(this._SQLCmd);
this.SetParamHT(adapter);
adapter.Fill(dataSet);
return dataSet;
}
if (this.Mode == QueryMode.ExecuteFunction)
{
SqlDataReader reader = this._SQLCmd.ExecuteReader();
while (reader.Read())
{
return reader[0];
}
return 0;
}
this._SQLCmd.ExecuteScalar();
adapter = new SqlDataAdapter(this._SQLCmd);
this.SetParamHT(adapter);
obj3 = true;
}
catch (SqlException exception)
{
ErrorLog.AddSqlException(exception);
obj3 = false;
}
catch (Exception exception2)
{
ErrorLog.AddException(exception2);
obj3 = false;
}
finally
{
if (connection != null)
{
connection.Close();
connection.Dispose();
}
}
return obj3;
}
private bool ExistsInHT(string key)
{
return this._ParamHT.ContainsKey(key);
}
public object GetPDataType()
{
object obj2;
SqlConnection connection = new SqlConnection(this._ConnectionString);
try
{
SqlConnection connection2 = new SqlConnection(this._ConnectionString);
SqlCommand selectCommand = new SqlCommand("select dbo.syscolumns.name,type_name(dbo.syscolumns.xusertype) as dt, dbo.syscolumns.isoutparam from dbo.sysobjects inner join dbo.syscolumns on dbo.syscolumns.id = dbo.sysobjects.id where dbo.sysobjects.name = '" + this._SPName + "'", connection2);
connection2.Open();
selectCommand.ExecuteNonQuery();
DataTable dataTable = new DataTable();
Hashtable hashtable = new Hashtable();
new SqlDataAdapter(selectCommand).Fill(dataTable);
connection2.Close();
connection2.Dispose();
if (dataTable.Rows.Count > 0)
{
foreach (DataRow row in dataTable.Rows)
{
hashtable.Add(row[0], row[1]);
}
}
obj2 = hashtable;
}
catch (SqlException exception)
{
ErrorLog.AddSqlException(exception);
obj2 = null;
}
catch (Exception exception2)
{
ErrorLog.AddException(exception2);
obj2 = null;
}
finally
{
if (connection != null)
{
connection.Dispose();
}
}
return obj2;
}
public void SetParam(string pName, string pDataType, string pValue, bool flag)
{
switch (pDataType.ToLower())
{
case "nvarchar":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.NVarChar, 0xfa0);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
case "varchar":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.VarChar, 0xfa0);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
case "text":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.Text);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
case "char":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.Char);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
case "int":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.Int);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
case "smallint":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.SmallInt);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
case "bigint":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.BigInt);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
case "numeric":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.Decimal, 0x26);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = Convert.ToDecimal(pValue);
}
break;
case "money":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.Money);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
case "decimal":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.VarChar, 0x26);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
case "float":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.Float);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = Convert.ToDouble(pValue);
}
break;
case "datetime":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.DateTime);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
case "date":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.DateTime);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
case "bit":
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.Bit);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
default:
this.SPParam = this._SQLCmd.Parameters.Add(pName, SqlDbType.VarChar, 0xfa0);
if (flag)
{
this.SPParam.Direction = ParameterDirection.Output;
}
if (pValue == "")
{
this.SPParam.Value = DBNull.Value;
}
else
{
this.SPParam.Value = pValue;
}
break;
}
}
private void SetParamHT(SqlDataAdapter sda)
{
try
{
for (int i = 0; i < sda.SelectCommand.Parameters.Count; i++)
{
string key = "#" + sda.SelectCommand.Parameters[i].ParameterName.Replace("@", "");
if (this.ExistsInHT(key))
{
this._ParamHT[key] = sda.SelectCommand.Parameters[i].Value.ToString();
}
}
}
catch (Exception exception)
{
ErrorLog.AddException(exception);
}
}
public string ConnectionString
{
get
{
return this._ConnectionString;
}
set
{
this._ConnectionString = value;
}
}
public QueryMode Mode
{
get
{
return this._QMode;
}
set
{
this._QMode = value;
}
}
public Hashtable ParamHT
{
get
{
return this._ParamHT;
}
set
{
this._ParamHT = value;
}
}
public string SPName
{
get
{
return this._SPName;
}
set
{
this._SPName = value;
}
}
public enum QueryMode
{
ExecuteFunction = 5,
ExecuteNonQuery = 1,
ExecuteReader = 2,
ExecuteScalar = 3,
ExecuteXmlReader = 4
}
}
1 Answer 1
You have 4 unused variables:
private string outputParams; private const string prfOut = "#"; private SqlDataReader SPDataReader; private SqlTransaction SPTransaction;
You can use object initializer in
ExecuteSP()
for your_SQLCmd
:_SQLCmd = new SqlCommand(_SPName, connection) {CommandType = CommandType.StoredProcedure};
There are 2 places where you do:
finally { if (connection != null) { connection.Close(); connection.Dispose(); } }
The if statement here is redundant.
Moving on to your longest method - SetParam
, most of your code here is redundant.
First this if statement is the same everywhere:
if (flag) { SPParam.Direction = ParameterDirection.Output; }
You can just move it to the bottom or the top.
Next this if statement is also the same everywhere:
if (pValue == "")
{
SPParam.Value = DBNull.Value;
}
But there is an else connected to it so you cant just move out this one without worrying about the else. What you can do instead is make a helper variable to save the correct value upon entering the case:
and than move the whole if/else outside the switch
.
Lastly I'm not sure if this is possible in your case but if you can convert pDataType
from string
to SqlDbType
your switch case will be just a few lines. Without that it looks like this:
public void SetParam(string pName, string pDataType, string pValue, bool flag)
{
object paramValue = pValue;
switch (pDataType.ToLower())
{
case "nvarchar":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.NVarChar, 0xfa0);
break;
case "varchar":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.VarChar, 0xfa0);
break;
case "text":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.Text);
break;
case "char":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.Char);
break;
case "int":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.Int);
break;
case "smallint":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.SmallInt);
break;
case "bigint":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.BigInt);
break;
case "numeric":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.Decimal, 0x26);
paramValue = Convert.ToDecimal(pValue);
break;
case "money":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.Money);
break;
case "decimal":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.VarChar, 0x26);
break;
case "float":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.Float);
paramValue = Convert.ToDouble(pValue);
break;
case "datetime":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.DateTime);
break;
case "date":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.DateTime);
break;
case "bit":
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.Bit);
break;
default:
SPParam = _SQLCmd.Parameters.Add(pName, SqlDbType.VarChar, 0xfa0);
break;
}
if (flag)
{
SPParam.Direction = ParameterDirection.Output;
}
SPParam.Value = string.IsNullOrEmpty(pValue) ? DBNull.Value : paramValue;
}
You can get rid of some of your private variables and shorten your properties by using auto properties:
private string _ConnectionString = string.Empty; private Hashtable _ParamHT = new Hashtable(); private QueryMode _QMode = QueryMode.ExecuteNonQuery; private string _SPName = string.Empty; public string ConnectionString { get { return _ConnectionString; } set { _ConnectionString = value; } } public QueryMode Mode { get { return _QMode; } set { _QMode = value; } } public Hashtable ParamHT { get { return _ParamHT; } set { _ParamHT = value; } } public string SPName { get { return _SPName; } set { _SPName = value; } }
Can be shorten to:
public string ConnectionString { get; set; } = string.Empty;
public QueryMode Mode { get; set; } = QueryMode.ExecuteNonQuery;
public Hashtable ParamHT { get; set; } = new Hashtable();
public string SPName { get; set; } = string.Empty;
-
\$\begingroup\$ If you parsed the
pDataType
you could get rid of the entireswitch
;-) but I wonder why OP is using strings for it at all and not just the enum. \$\endgroup\$t3chb0t– t3chb0t2017年03月26日 18:45:55 +00:00Commented Mar 26, 2017 at 18:45 -
\$\begingroup\$ @t3chb0t Well in case he obtains the data from somewhere else not manually passing it, it makes some sense to be a string but still, it should simply be the enum type. \$\endgroup\$Denis– Denis2017年03月26日 19:07:24 +00:00Commented Mar 26, 2017 at 19:07