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();