5
\$\begingroup\$

I am going through one of my class library projects, and while nothing is wrong with it, I am finding myself being a bit anal, and wanting to organize things a bit differently.

The project in particular that I am looking at reorganizing, is a class library to hook into a multitude of databases, and do work on them. Now because of the differences in datatypes and methods for doing said work, each database has its own sub-folder in the main project, labelled after the database it will do the work for.

Would it be best to keep it like it is, or combine the similar methods into 1 class and simply through a switch statement in, giving the option to "select" the database type the library should be working with?

Right now it's setup as you simply use or import My.ClassLibrary.MSSQL and the like, however, all the methods in each type are so flippin similar, I am looking at it and saying to myself... why?

Each Database type has a folder in this project containing the database specific representation of the following code:

Access Class

using System;
using System.Data;
using System.Data.SqlClient;
using System.Threading.Tasks;
namespace kp.Class.Library.Data
{
 internal class Access : IDisposable
 {
#region "Properties"
 // Set the type of query we are running
 private CommandType _QT;
 internal CommandType QueryType { set { _QT = value; } }
 // Set the actual query text to run
 private string _Qry;
 internal string Query { set { _Qry = value; } }
 // Set the parameter names if there are any
 private string[] _PNs;
 internal string[] ParameterNames { set { _PNs = value; } }
 // Set the parameter values if there are any
 private object[] _PVs;
 internal object[] ParameterValues { set { _PVs = value; } }
 // Set the actual Sql Data Types if there are any
 private DataTypes[] _DTs;
 internal DataTypes[] ParameterDataTypes { set { _DTs = value; } }
 // Check to see if there are any parameters passed
 private bool AreParams() {
 // Check to see if the values and names are null first
 if (_PVs != null && _PNs != null) {
 try {
 Type _t_pv = _PVs.GetType();
 Type _t_pn = _PNs.GetType();
 if (_t_pv.IsArray && _t_pn.IsArray) {
 return (_PVs.Length > 0 && _PNs.Length > 0) ? true : false;
 } else {
 return false;
 }
 } catch {
 // yes I meant to do this, we really don't need to get the exception here
 return false;
 }
 } else {
 return false;
 }
 }
 // Get a return message if any
 private string _Msg;
 internal string Message { get { return _Msg; } }
 // Set the official Sql Reader object
 private SqlDataReader _Rdr;
 // Set the official Sql Connection object
 private SqlConnection _Conn;
 // Set the official Sql Command object
 private SqlCommand _Cmd;
 // Hack for seeing if we're disposed already
 private bool disposedValue;
#endregion
 // Constructor
 internal Access(string _connStr)
 {
 Invoke(_connStr);
 }
 // Official Constructor. We can thread these 2 becuase they are not being used yet, and it makes it slightly more efficient
 internal void Invoke(string _connStr)
 {
 try { 
 Parallel.Invoke(() => {
 _Conn = new SqlConnection(_connStr);
 }, () =>
 {
 _Cmd = new SqlCommand();
 });
 }
 catch (SqlException dEx)
 {
 // Catch an exception if any, an write it out to our logging mechanism, in addition to adding it our returnable message property
 _Msg = "Access.Invoke Exception: " + dEx.Message;
 ErrorReporting.WriteEm.WriteItem(dEx, "kp.Class.Library.Data.Access.Invoke", _Msg);
 }
 catch (Exception ex)
 {
 _Msg = "Access.Invoke Exception: " + ex.Message;
 ErrorReporting.WriteEm.WriteItem(ex, "kp.Class.Library.Data.Access.Invoke", _Msg);
 }
 }
 /// <summary>
 /// Return a SqlDataReader based on the properties passed to this class
 /// </summary>
 /// <returns></returns>
 internal SqlDataReader GetResults()
 {
 try {
 // check for parameters
 if (AreParams()) {
 PrepareParams(_Cmd);
 }
 // set our connection
 _Cmd.Connection = _Conn;
 // set the type of query to run
 _Cmd.CommandType = _QT;
 // set the actual query to run
 _Cmd.CommandText = _Qry;
 // open the connection
 _Cmd.Connection.Open();
 // prepare the command with any parameters that may have gotten added
 _Cmd.Prepare();
 // Execute the SqlDataReader, and set the connection to close once returned
 _Rdr = _Cmd.ExecuteReader(CommandBehavior.CloseConnection);
 // clear out any parameters
 _Cmd.Parameters.Clear();
 // return our reader object
 return (!_Rdr.HasRows) ? null : _Rdr;
 }
 catch (SqlException SqlEx)
 {
 _Msg += "Acccess.GetResults SqlException: " + SqlEx.Message;
 ErrorReporting.WriteEm.WriteItem(SqlEx, "kp.Class.Library.Data.Access.GetResults", _Msg);
 return null;
 }
 catch (Exception ex) {
 _Msg += "Acccess.GetResults Exception: " + ex.Message;
 ErrorReporting.WriteEm.WriteItem(ex, "kp.Class.Library.Data.Access.GetResults", _Msg);
 return null;
 }
 }
 /// <summary>
 /// Execute a non-return query, and return the success
 /// </summary>
 /// <returns></returns>
 internal bool Execute() {
 try {
 // check for parameters
 if (AreParams()) {
 PrepareParams(_Cmd);
 }
 // set our connection
 _Cmd.Connection = _Conn;
 // set the type of query to run
 _Cmd.CommandType = _QT;
 // set the actual query to run
 _Cmd.CommandText = _Qry;
 // open the connection
 _Cmd.Connection.Open();
 // prepare the command with any parameters that may have gotten added
 _Cmd.Prepare();
 // execute the non-returnable query against the database
 _Cmd.ExecuteNonQuery();
 // clear out any parameters
 _Cmd.Parameters.Clear();
 // executed successfully (otherwise would have thrown an exception)
 return true;
 }
 catch (SqlException SqlEx)
 {
 _Msg += "Access.Execute SqlException: " + SqlEx.Message;
 ErrorReporting.WriteEm.WriteItem(SqlEx, "kp.Class.Library.Data.Access.Execute", _Msg);
 return false;
 }
 catch (Exception ex) {
 _Msg += "Access.Execute Exception: " + ex.Message;
 ErrorReporting.WriteEm.WriteItem(ex, "kp.Class.Library.Data.Access.Execute", _Msg);
 return false;
 }
 }
 /// <summary>
 /// Execute a query with a return value. Used in Selecting the ID of the last inserted record.
 /// </summary>
 /// <typeparam name="T"></typeparam>
 /// <param name="_DefVal"></param>
 /// <returns></returns>
 internal T ExecuteWithReturn<T>(T _DefVal) {
 try {
 T _Ret;
 // check for parameters
 if (AreParams()) {
 PrepareParams(_Cmd);
 }
 // set our connection
 _Cmd.Connection = _Conn;
 // set the type of query to run
 _Cmd.CommandType = _QT;
 // set the actual query to run
 _Cmd.CommandText = _Qry;
 // open the connection
 _Cmd.Connection.Open();
 // prepare the command with any parameters that may have gotten added
 _Cmd.Prepare();
 T _T = (T)_Cmd.ExecuteScalar();
 _Ret = (_T is DBNull) ? default(T) : _T;
 // clear out _T
 _T = default(T);
 // clear out any parameters
 _Cmd.Parameters.Clear();
 // return the single return value from the query run
 return _Ret;
 }
 catch (SqlException SqlEx)
 {
 _Msg += "Access.ExecuteWithReturn SqlException: " + SqlEx.Message;
 ErrorReporting.WriteEm.WriteItem(SqlEx, "kp.Class.Library.Data.Access.ExecuteWithReturn", _Msg);
 return default(T);
 } catch (Exception ex) {
 _Msg += "Access.ExecuteWithReturn Exception: " + ex.Message;
 ErrorReporting.WriteEm.WriteItem(ex, "kp.Class.Library.Data.Access.ExecuteWithReturn", _Msg);
 return default(T);
 }
 }
 /// <summary>
 /// Prepare our parameters, adding them and forcing a valid data length
 /// </summary>
 /// <param name="objCmd"></param>
 protected void PrepareParams(SqlCommand objCmd) {
 try {
 // set our initial Data Size
 int _DataSize = 0;
 // get the number of Parameter Values passed in
 int _PCt = _PVs.GetUpperBound(0);
 // begin array check
 Type _t_dt = _DTs.GetType();
 // start looping over our parameters
 for (int i = 0; i <= _PCt; ++i) {
 // make sure that the data types are actually an array
 if (_t_dt.IsArray) {
 // select which datatype, and force the official size
 switch ((int)_DTs[i]) {
 case 0:
 case 33:
 case 6:
 case 9:
 case 13:
 case 19:
 _DataSize = 8;
 break;
 case 1:
 case 3:
 case 7:
 case 10:
 case 12:
 case 21:
 case 22:
 case 23:
 case 25:
 _DataSize = _PVs[i].ToString().Length;
 break;
 case 2:
 case 20:
 _DataSize = 1;
 break;
 case 5:
 _DataSize = 17;
 break;
 case 8:
 case 17:
 case 15:
 _DataSize = 4;
 break;
 case 14:
 _DataSize = 16;
 break;
 case 31:
 _DataSize = 3;
 break;
 case 32:
 _DataSize = 5;
 break;
 case 16:
 _DataSize = 2;
 break;
 }
 // add our parameter to the command object
 objCmd.Parameters.Add(_PNs[i], (SqlDbType)_DTs[i], _DataSize).Value = _PVs[i]; 
 } else {
 // if the datatypes were not set, try to add them generically
 objCmd.Parameters.AddWithValue(_PNs[i], _PVs[i]);
 }
 }
 // clean up
 _PNs = null;_PVs = null;_DTs = null;
 } catch (Exception ex) {
 _Msg += "Access.PrepareParams Exception: " + ex.Message;
 ErrorReporting.WriteEm.WriteItem(ex, "kp.Class.Library.Data.Access.PrepareParams", _Msg);
 }
 }
#region "Dispose Support"
 protected virtual void Dispose(bool disposing)
 {
 if (!disposedValue && disposing) {
 try
 {
 _Qry = string.Empty;
 _Rdr.Close();
 _Rdr.Dispose();
 _Cmd.Connection.Close();
 _Cmd.Dispose();
 if (_Conn.State == ConnectionState.Open)
 {
 SqlConnection.ClearAllPools();
 _Conn.Close();
 _Conn.Dispose();
 }
 _Msg = null;
 }
 catch(Exception ex) {
 ErrorReporting.WriteEm.WriteItem(ex, "kp.Class.Library.Data.Access.Dispose", "");
 }
 }
 disposedValue = true;
 }
 public void Dispose()
 {
 Dispose(true);
 GC.SuppressFinalize(this);
 }
#endregion
 }
}

Wrapper Class

using System;
using System.Collections.Generic;
using System.Data;
using System.Data.Common;
namespace kp.Class.Library.Data
{
 /// <summary>
 /// Wrapper class for our data access
 /// </summary>
 public class Wrapper {
 /// <summary>
 /// Setup our return message if any
 /// </summary>
 public static string Message { get { return _Msg; } }
 private static string _Msg;
 // Instantiate our caching methods
 internal static Common.CustomCache _Cache = new Common.CustomCache();
 // Map our datareader object to a strongly typed list
 private static IList<T> Map<T>(DbDataReader dr) where T : new()
 {
 try
 {
 // initialize our returnable list
 List<T> list = new List<T>();
 // fire up the lamda mapping
 var converter = new Converter<T>(dr);
 while (dr.Read())
 {
 // read in each row, and properly map it to our T object
 var obj = converter.CreateItemFromRow();
 // add it to our list
 list.Add(obj);
 }
 // reutrn it
 return list;
 }
 catch (Exception ex)
 {
 // Catch an exception if any, an write it out to our logging mechanism, in addition to adding it our returnable message property
 _Msg += "Wrapper.Map Exception: " + ex.Message;
 ErrorReporting.WriteEm.WriteItem(ex, "kp.Class.Library.Data.Wrapper.Map", _Msg);
 // make sure this method returns a default List
 return default(List<T>);
 }
 }
 /// <summary>
 /// Get the results of a stronly-typed IList Object
 /// </summary>
 /// <typeparam name="T">Strongly-Typed class of objects that should be returned</typeparam>
 /// <param name="_connStr">The connection string to the database</param>
 /// <param name="_Qry">The query to run</param>
 /// <param name="_QryType">The Query Type to run</param>
 /// <param name="_ParamNames">The Parameters' names to pass to the query, if any</param>
 /// <param name="_ParamVals">The Parameters' values to pass to the query, if any</param>
 /// <param name="_ParamDTs">The Parameters' data types to pass to the query, if any</param>
 /// <param name="_ShouldCache">Should we cache the response</param>
 /// <param name="_CacheID">Cache item name</param>
 /// <returns>Strongly Typed ilist of objects</returns>
 public static IList<T> GetResults<T>(string _connStr, string _Qry, CommandType _QryType,
 string[] _ParamNames = null,
 object[] _ParamVals = null,
 DataTypes[] _ParamDTs = null,
 bool _ShouldCache = false,
 string _CacheID = "") where T : new()
 {
 // Create a reference to a potential already cached IList
 IList<T> _CachedItem = _Cache.Get<IList<T>>(_CacheID);
 // If we're already cached, there's no need to fire up the data access objects, so return the cached item instead
 if (_CachedItem != null && _ShouldCache)
 {
 return _CachedItem;
 }
 else
 {
 // Fire up our data access object
 using (Access db = new Access(_connStr))
 {
 try
 {
 // create a new ilist reference of our strongly typed class
 IList<T> _Query = default(IList<T>);
 // set the query type
 db.QueryType = _QryType;
 // set the query text
 db.Query = _Qry;
 // make sure we've got some parameters, if we do the set them to our db access object
 if (_ParamNames != null)
 {
 // set the parameter names
 db.ParameterNames = _ParamNames;
 // set the parameter values
 db.ParameterValues = _ParamVals;
 // set the parameter data types
 db.ParameterDataTypes = _ParamDTs;
 }
 // start using our db access :) Fire off the GetResults method and return back a SqlDataReader to work on
 using (DbDataReader r = db.GetResults())
 {
 // make sure the data reader actually exists and contains some results
 if (r != null)
 {
 // map the data reader to our strongly type(s)
 _Query = Map<T>(r);
 }
 r.Close();
 }
 // check if we should cache the results
 if (_ShouldCache)
 {
 // if so, set the query object to the cache
 _Cache.Set<IList<T>>(_Query, _CacheID);
 }
 // return our strongly typed list
 return _Query;
 }
 catch (DbException dEx)
 {
 // Catch an exception if any, an write it out to our logging mechanism, in addition to adding it our returnable message property
 _Msg += "Wrapper.GetResults Exception: " + dEx.Message + db.Message;
 ErrorReporting.WriteEm.WriteItem(dEx, "kp.Class.Library.Data.Wrapper.GetResults", _Msg);
 // make sure this method returns a default List
 return default(IList<T>);
 }
 catch (Exception ex)
 {
 // Catch an exception if any, an write it out to our logging mechanism, in addition to adding it our returnable message property
 _Msg += "Wrapper.GetResults Exception: " + ex.Message + db.Message;
 ErrorReporting.WriteEm.WriteItem(ex, "kp.Class.Library.Data.Wrapper.GetResults", _Msg);
 // make sure this method returns a default List
 return default(IList<T>);
 }
 }
 }
 }
 /// <summary>
 /// Execute a query against the database. Usually used for IUD Operations
 /// </summary>
 /// <param name="_connStr">The connection string to the database</param>
 /// <param name="_Qry">The query to run</param>
 /// <param name="_QryType">The Query Type to run</param>
 /// <param name="_ParamNames">The Parameters' names to pass to the query, if any</param>
 /// <param name="_ParamVals">The Parameters' values to pass to the query, if any</param>
 /// <param name="_ParamDTs">The Parameters' data types to pass to the query, if any</param>
 /// <returns>Boolean of success</returns>
 public static bool Execute(string _connStr, string _Qry, CommandType _QryType,
 string[] _ParamNames = null,
 object[] _ParamVals = null,
 DataTypes[] _ParamDTs = null)
 {
 // setup a reference for our success return
 bool _T;
 // Fire up our data access object
 using (Access db = new Access(_connStr))
 {
 try {
 // set the query type
 db.QueryType = _QryType;
 // set the query text
 db.Query = _Qry;
 // make sure we've got some parameters, if we do the set them to our db access object
 if (_ParamNames != null)
 {
 // set the parameter names
 db.ParameterNames = _ParamNames;
 // set the parameter values
 db.ParameterValues = _ParamVals;
 // set the parameter data types
 db.ParameterDataTypes = _ParamDTs;
 }
 // execute the query and return if it was successful or not
 _T = db.Execute();
 // return it
 return _T;
 }
 catch (DbException dEx)
 {
 // Catch an exception if any, an write it out to our logging mechanism, in addition to adding it our returnable message property
 _Msg += "Wrapper.Execute Exception: " + dEx.Message + db.Message;
 ErrorReporting.WriteEm.WriteItem(dEx, "kp.Class.Library.Data.Wrapper.Execute", _Msg);
 // make sure this method returns a default List
 return false;
 }
 catch (Exception ex)
 {
 // Catch an exception if any, an write it out to our logging mechanism, in addition to adding it our returnable message property
 _Msg += "Wrapper.Execute Exception: " + ex.Message + db.Message;
 ErrorReporting.WriteEm.WriteItem(ex, "kp.Class.Library.Data.Wrapper.Execute", _Msg);
 // make sure this method returns a default value of false
 return false;
 }
 }
 }
 /// <summary>
 /// Executes a query against the database, and returns a value
 /// </summary>
 /// <typeparam name="T">Strongly Typed Object for return</typeparam>
 /// <param name="_connStr">The connection string to the database</param>
 /// <param name="_Qry">The query to run</param>
 /// <param name="_QryType">The Query Type to run</param>
 /// <param name="_ParamNames">The Parameters' names to pass to the query, if any</param>
 /// <param name="_ParamVals">The Parameters' values to pass to the query, if any</param>
 /// <param name="_ParamDTs">The Parameters' data types to pass to the query, if any</param>
 /// <param name="_DefVal">Default value that should get returned if none are</param>
 /// <returns>Strongly Typed object from the query executed</returns>
 public static T ExecuteWithReturn<T>(string _connStr, string _Qry, CommandType _QryType,
 string[] _ParamNames = null,
 object[] _ParamVals = null,
 DataTypes[] _ParamDTs = null,
 object _DefVal = null) where T : new() {
 // setup a new reference to T
 T _T;
 // Fire up our data access object
 using (Access db = new Access(_connStr))
 {
 try{
 // set the query type
 db.QueryType = _QryType;
 // set the query text
 db.Query = _Qry;
 // make sure we've got some parameters, if we do the set them to our db access object
 if (_ParamNames != null)
 {
 // set the parameter names
 db.ParameterNames = _ParamNames;
 // set the parameter values
 db.ParameterValues = _ParamVals;
 // set the parameter data types
 db.ParameterDataTypes = _ParamDTs;
 }
 // execute the query and return the results back to _T
 _T = db.ExecuteWithReturn<T>((T)_DefVal);
 // return it
 return (_T is DBNull) ? default(T) : _T;
 }
 catch (DbException dEx)
 {
 // Catch an exception if any, an write it out to our logging mechanism, in addition to adding it our returnable message property
 _Msg += "Wrapper.ExecuteWithReturn Exception: " + dEx.Message + db.Message;
 ErrorReporting.WriteEm.WriteItem(dEx, "kp.Class.Library.Data.Wrapper.ExecuteWithReturn", _Msg);
 // make sure this method returns a default List
 return default(T);
 }
 catch (Exception ex)
 {
 // Catch an exception if any, an write it out to our logging mechanism, in addition to adding it our returnable message property
 _Msg += "Wrapper.ExecuteWithReturn Exception: " + ex.Message + db.Message;
 ErrorReporting.WriteEm.WriteItem(ex, "kp.Class.Library.Data.Wrapper.ExecuteWithReturn", _Msg);
 // return the default value for the strong typed object
 return default(T);
 }
 }
 }
 }
}

Data Typing Class

namespace kp.Class.Library.Data.SqlServer
{
 /// <summary>
 /// Sql Data Type Enumeration
 /// </summary>
 public enum DataTypes : int
 {
 /// <summary>
 /// BigInt
 /// </summary>
 BigInt = 0,
 /// <summary>
 /// Binary
 /// </summary>
 Binary = 1,
 /// <summary>
 /// Bit
 /// </summary>
 Bit = 2,
 /// <summary>
 /// Char
 /// </summary>
 Char = 3,
 /// <summary>
 /// Date
 /// </summary>
 Date = 31,
 /// <summary>
 /// DateTime
 /// </summary>
 DateTime = 4,
 /// <summary>
 /// DateTime2
 /// </summary>
 DateTime2 = 33,
 /// <summary>
 /// DateTimeOffset
 /// </summary>
 DateTimeOffset = 34,
 /// <summary>
 /// Decimal
 /// </summary>
 Decimal = 5,
 /// <summary>
 /// Float
 /// </summary>
 Float = 6,
 /// <summary>
 /// Image
 /// </summary>
 Image = 7,
 /// <summary>
 /// Int
 /// </summary>
 Int = 8,
 /// <summary>
 /// Money
 /// </summary>
 Money = 9,
 /// <summary>
 /// NChar
 /// </summary>
 NChar = 10,
 /// <summary>
 /// NText
 /// </summary>
 NText = 11,
 /// <summary>
 /// NVarChar
 /// </summary>
 NVarChar = 12,
 /// <summary>
 /// Real
 /// </summary>
 Real = 13,
 /// <summary>
 /// SmallDateTime
 /// </summary>
 SmallDateTime = 15,
 /// <summary>
 /// SmallInt
 /// </summary>
 SmallInt = 16,
 /// <summary>
 /// SmallMoney
 /// </summary>
 SmallMoney = 17,
 /// <summary>
 /// Structured
 /// </summary>
 Structured = 30,
 /// <summary>
 /// Text
 /// </summary>
 Text = 18,
 /// <summary>
 /// Time
 /// </summary>
 Time = 32,
 /// <summary>
 /// Timestamp
 /// </summary>
 Timestamp = 19,
 /// <summary>
 /// TinyInt
 /// </summary>
 TinyInt = 20,
 /// <summary>
 /// Udt
 /// </summary>
 Udt = 29,
 /// <summary>
 /// UniqueIdentifier
 /// </summary>
 UniqueIdentifier = 14,
 /// <summary>
 /// VarBinary
 /// </summary>
 VarBinary = 21,
 /// <summary>
 /// VarChar
 /// </summary>
 VarChar = 22,
 /// <summary>
 /// Variant
 /// </summary>
 Variant = 23,
 /// <summary>
 /// Xml
 /// </summary>
 Xml = 25
 }
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Nov 3, 2014 at 15:32
\$\endgroup\$
0

2 Answers 2

11
\$\begingroup\$

Would it be best to keep it like it is, or combine the similar methods into 1 class and simply through a switch statement in, giving the option to "select" the database type the library should be working with?

This is a two part question. Where I am only refering to

Part1

Would it be best to keep it like it is, or combine the similar methods into 1 class ?

This is where the design principle DRY - don't repeat yourself would come in handy. Also, I am not a big fan of inheritance (I prefer composition), this would be a perfect candidate to build a abstract or must inherit class which should implement all the non database system specific code.

So a Access class will inherit this super class and will add it's own system specific part to the class. So e.g for Access or any OleDB related operation, the parameters of a parameterized query just use the ? as placeholder, but must ensure the order of the parameters.

General

You code seems to be too commented. For every setting/assignment you first write the comment what you are doing and then in the next line the code itself.

Some examples

// check if we should cache the results
if (_ShouldCache) 
// set the parameter data types
db.ParameterDataTypes = _ParamDTs;

This is blowing your codebase up to 1000 lines. Renaming your parameters and fields to something more meaningful would reduce the comments to the minimum.

Comments should be used to explain why something is done. What is done by the code should be documented by the code itself.

So by renaming _ShouldCache to shouldResultsBeCached (I don't like the underscores) and ParamDTs to parameterDataTypes you could remove the comments and Mr.Maintainer still would understand what your code does.

Access class

The // select which datatype, and force the official size part of the PrepareParams() method should be extracted to it's own method.

This tenary expression/operator

 return (_PVs.Length > 0 && _PNs.Length > 0) ? true : false; 

can be expressed like

return (_PVs.Length > 0 && _PNs.Length > 0); 

which is more clear. This came form the method AreParams()

private bool AreParams() {
 // Check to see if the values and names are null first
 if (_PVs != null && _PNs != null) {
 try {
 Type _t_pv = _PVs.GetType();
 Type _t_pn = _PNs.GetType();
 if (_t_pv.IsArray && _t_pn.IsArray) {
 return (_PVs.Length > 0 && _PNs.Length > 0) ? true : false;
 } else {
 return false;
 }
 } catch {
 // yes I meant to do this, we really don't need to get the exception here
 return false;
 }
 } else {
 return false;
 }
} 

If you use a guard condition, your code will be indented less. Adding up condition checks can remove if..else statements.
The method can be refactored and renamed to

 private bool HasParameters() {
 // Check to see if the values and names are null first
 if (_PVs == null || _PNs == null) { return false;}
 try {
 Type _t_pv = _PVs.GetType();
 Type _t_pn = _PNs.GetType();
 return (_t_pv.IsArray && _t_pn.IsArray && _PVs.Length > 0 && _PNs.Length > 0);
 }
 } catch {
 // yes I meant to do this, we really don't need to get the exception here
 return false;
 }
 }

Using auto properties (where possible) can simplify your code also, as you name your properties meaningful, but not your backing fields.

Example

private CommandType _QT;
internal CommandType QueryType { set { _QT = value; } }

could become

internal CommandType QueryType { private get; set; }

Wrapper class

Inside your ExecuteWithReturn() method you are catching 2 different exception types, which you treat exactly the same (only the exception variable name differs). If you treat exceptions the same, you don't need to distinguish between them.

Style

Be consistent with the style you use. Try to use style the majority uses so a new developer will be used to it.

Example

Sometimes you put a opening brace { to a new line (where it belongs) sometimes at the end of the line.

answered Nov 3, 2014 at 16:44
\$\endgroup\$
5
  • \$\begingroup\$ Thank you very much for this. It is a convert from an older VB.Net library that I inherited, however, I am still very green to c# \$\endgroup\$ Commented Nov 3, 2014 at 18:00
  • \$\begingroup\$ +1 if for nothing other than: "Comments should be used to explain why something is done. What is done by the code should be documented by the code itself." \$\endgroup\$ Commented Nov 5, 2014 at 22:11
  • \$\begingroup\$ private get is a very odd thing to do. It very rarely makes the API better (maybe in a security situation, like a password field). For the most part, if you can set a property there's no good reason to hide it's value after setting. IMHO it's a violation of the Principle of least astonishment \$\endgroup\$ Commented Nov 7, 2014 at 5:54
  • \$\begingroup\$ @craftworkgames Instead of commenting on this, why don't you write an answer ? Here I showed how a former readonly property could be expressed using auto properties. As the value is needed by the class, at the minimum a private get is needed too. \$\endgroup\$ Commented Nov 7, 2014 at 6:57
  • \$\begingroup\$ @Heslacher yeah your right I could write an answer. I didn't think I had enough to elaborate on though. Your answer is great, I've upvoted it. It was just a thought as I read through the code. \$\endgroup\$ Commented Nov 7, 2014 at 7:22
1
\$\begingroup\$

I'm a bit baffled by the whole Access class, which seems to needlessly reinvent things that have been available in .NET for years. Much of its comments are superfluous -- // open the connection -- and it really should be rewritten. The "detect datatype switch" in PrepareParams is utterly baffling, I fail to see why you'd use integers instead of SqlDbType.

This code looks ancient, a relic from the 1.1 or 2.0 days. I'd recommend ditching it and implementing a modern ORM (NHibernate, Entity Framework,...).

answered Nov 3, 2014 at 16:41
\$\endgroup\$
1
  • \$\begingroup\$ These are my thoughts, too. \$\endgroup\$ Commented Nov 3, 2014 at 19:38

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.