Create database connection and run the insert, delete, update queries class-Update

Question

Based on the feedback I have got from the guys reviewing the code as posted in my previous question.

Create database connection and run the insert, delete, update queries class

I have used interfaces and wrote the following code

/* */

namespace SampleInterfaceT1
{
public interface IDbConSetup
{
 DbConnection OpenConnection(string connectionString);
 void CloseConnection();
}
public interface ISqlDataProvider : IDbConSetup
{
 int ExecuteCmd(string dbCmd, SqlParameter param);
}
public class SqlClient : ISqlDataProvider
{
 private SqlConnection _dbconnect;
 private string _conString;
 public DbConnection OpenConnection(string connectionString)
 {
 if (String.IsNullOrWhiteSpace(connectionString))
 {
 throw new ArgumentException("Invalid Connection String");
 }
 _conString = connectionString;
 Console.WriteLine("Sql Connection Opened.");
 _dbconnect = new SqlConnection(_conString);
 _dbconnect.Open();
 return _dbconnect;
 }
 public int ExecuteCmd(string dbCmd, SqlParameter param)
 {
 int _rc;
 if (String.IsNullOrWhiteSpace(dbCmd))
 {
 throw new ArgumentException("Invalid Connection String");
 }
 using (SqlCommand _sqlCmd = new SqlCommand(dbCmd, _dbconnect))
 {
 _sqlCmd.CommandType = CommandType.Text;
 _sqlCmd.Parameters.Add(param);
 _rc = _sqlCmd.ExecuteNonQuery();
 }
 Console.WriteLine("Command executed");
 return _rc;
 }
 public void CloseConnection()
 {
 _dbconnect.Close();
 Console.WriteLine("Sql Connection Closed.");
 _dbconnect.Dispose();
 }
 }
}
/* Main Program execution */
namespace SampleInterfaceT1
{
 public class Program
 {
 private static void Main(string[] args)
 {
 string conString = @""; /* Connection String*/
 string dbCommand = @"INSERT INTO [dbo].[TestTable]([Name]) VALUES (@Name)";
 ISqlDataProvider SqlDataProviderObject = new SqlClient();
 SqlDataProviderObject.OpenConnection(conString);
 SqlDataProviderObject.ExecuteCmd(dbCommand, new SqlParameter("Name", "HellofromnewDictionary"));
 SqlDataProviderObject.CloseConnection();
 Console.ReadLine();
 }
}
}

I'm a newbie to the concept of interfaces. I'm I on the right path or did I got it completely wrong?

Answer 1

You won't gain any advantage by using abbreviations for your parameter/variable names but you will loose readability. Therefor I would like to suggest to use more descriptive names. Otherwise you will come back in a couple of weeks/months and you will be wondering what e.g int _rc; should represent.

This

if (String.IsNullOrWhiteSpace(dbCmd))
{
 throw new ArgumentException("Invalid Connection String");
}

in your ExecuteCmd() method shows why copy&pasta should be done in a wise way.

In addition you never check if the passed in SqlParameter para is null. In a public method you should always validate all passed in parameters.

As DbConnection is implementing IDisposable you should enclose its usage into a using block. This would also remove the need to have a CloseConnection() method. Hence this would result in removing the IDbConSetup interface.

Instead of having a public OpenConnection method you could pass the connection string to the constructor and make the method private. In addition you won't really need the class level field private SqlConnection _dbconnect;.

Methodlevel variables shouldn't be prefixed with an underscore. This is usually only done for class level variables.

A sample of a simplified DAL which still needs to be expanded.

Note this isn't tested

using System;
using System.Collections.Generic;
using System.Data;
using System.Data.Common;
public class SimplifiedDal
{
 private readonly DbProviderFactory dbFactory;
 private readonly string connectionString;
 public SimplifiedDal(string connectionString, string providerFactory)
 {
 if (string.IsNullOrWhiteSpace(connectionString))
 {
 throw new ArgumentException("Value may not be null or whitespace", "connectionString");
 }
 if (string.IsNullOrWhiteSpace(providerFactory))
 {
 throw new ArgumentException("Value may not be null or whitespace", "providerFactory");
 }
 dbFactory = DbProviderFactories.GetFactory(providerFactory);
 this.connectionString = connectionString;
 }
 public int ExecuteNonQuery(string sql, CommandType commandType, Dictionary<string, object> parameters)
 {
 if (string.IsNullOrWhiteSpace(sql))
 {
 throw new ArgumentException("Value may not be null or whitespace", "sql");
 }
 using (var connection = dbFactory.CreateConnection())
 {
 connection.ConnectionString = connectionString;
 connection.Open();
 using (var command = connection.CreateCommand())
 {
 AddParameters(command, parameters);
 return command.ExecuteNonQuery();
 }
 }
 }
 private void AddParameters(DbCommand command, Dictionary<string, object> parameters)
 {
 if (parameters == null || parameters.Count == 0) { return; }
 string dbFactoryName = dbFactory.GetType().Name;
 if (dbFactoryName == "OleDbFactory" || dbFactoryName == "OdbcFactory")
 {
 AddOleDbOdbcParameters(command, parameters);
 return;
 }
 string parameterPrefix = GetPrefixByFactoryName(dbFactoryName);
 foreach (var kvp in parameters)
 {
 var parameter = command.CreateParameter();
 parameter.ParameterName = parameterPrefix + kvp.Key;
 parameter.Value = kvp.Value;
 command.Parameters.Add(parameter);
 }
 }
 private string GetPrefixByFactoryName(string factoryName)
 {
 switch (factoryName)
 {
 case "SqlClientFactory":
 return "@";
 case "OracleClientFactory":
 return ":";
 case "MySqlClientFactory":
 return "?";
 }
 throw new NotImplementedException();
 }
 private void AddOleDbOdbcParameters(DbCommand command, Dictionary<string, object> parameters)
 {
 foreach (var kvp in parameters)
 {
 var parameter = command.CreateParameter();
 parameter.ParameterName = "?";
 parameter.Value = kvp.Value;
 command.Parameters.Add(parameter);
 }
 }
} 

which can be used like so

//MS Access
var msConnectionString = "A MS Access connection string";
var msDal = new SimplifiedDal(msConnectionString, "System.Data.OleDb");
//MySql
var mysqlConnectionString = "A MySQL connection string";
var mysqlDal = new SimplifiedDal(mysqlConnectionString, "System.Data.OleDb");
//MS Sql
var sqlConnectionString = @"A Sql connection string";
var sqlDal = new SimplifiedDal(sqlConnectionString, "System.Data.SqlClient"); 

you can easily check which factories are installed by using

DataTable table = DbProviderFactories.GetFactoryClasses();

• t3chb0t: This is an improvement but still... the three providers are hard-coded in the switch ;-] and... you rely on automatic data type mapping (I cannot find the SO question now but it can go wrong) - you should always use the type explicitly when setting parameters. IMO it's an overkill to abstract every bit of the providers and the switch... well it's a no-go as far as SOLID is concerned. As I said, it's a lot of work to make it really good ;-)
• t3chb0t: This is my answer ;-)