-3

I am using a Sql-connection to create a temporary table and perform DML operations on it. The temporary table act as a staging area, for preparation of data to be loaded into a data warehouse. I am using a using statement, to properly dispose of the Sql-connection after the program finishes (or fails), like the following:

using IDbConnection sqlCon = new SqlConnection(connectionString);
sqlCon.Open();
sqlCon.Execute("SELECT 5 AS Number INTO ##TempTable");
sqlCon.Execute("INSERT INTO ##TempTable SELECT 10 AS Number");       

In reality my queries are more complex than this and takes some effort to prepare. I would like to divide the associated work into more than one class, to work in an object-oriented fashion. That could be done by creating the following classing :

public class TableCreator
{
    private IDbConnection SqlCon { get; set; }
    
    public TableCreator(IDbConnection conn)
    {
        SqlCon = conn;
    }

    public void CreateTable()
    {
        sqlCon.Execute("SELECT 5 AS Number INTO ##TempTable");
    }
}

public class DataInserter
{
    private IDbConnection SqlCon { get; set; }
    
    public DataInserter(IDbConnection conn)
    {
        SqlCon = conn;
    }

    public void InsertData()
    {
        sqlCon.Execute("INSERT INTO ##TempTable SELECT 10 AS Number");
    }
}

In the main method I can then simply instantiate these classes and run their methods like this:

using IDbConnection sqlCon = new SqlConnection(connectionString);
sqlCon.Open();
TableCreator creator = new TableCreator(sqlCon);
stager.CreateTable();
DataInserter inserter = new DataInserter (sqlCon);
inserter.InsertData();

This makes the code clean, and easy to read. But it's probably not a good idea to be passing open connections into other classes or methods. There probably exists a better way to achieve the same.

I hope that any of you can give me some advice on this challenge. Is there a way that I can split the execute commands into separate classes, without passing the connection around as a parameter?

Mads
  • 17
  • 1
  • 6
  • 1
    Ditch `SqlAccess`. It's providing absolutely no value. – madreflection May 23 '22 at 15:43
  • The class is more complex than what I have shown in this simplified version. Nevertheless, that is a valid suggestion to consider. – Mads May 23 '22 at 15:59
  • 1
    You don't need that extra layer *in addition to Dapper*. – madreflection May 23 '22 at 15:59
  • @madreflection you are correct, I think you have helped me to narrow down my issue. Please see my edit. – Mads May 23 '22 at 16:41
  • 1
    So you made a wrapper which castrates Dapper and now you're complaining that you cannot do what you want? You made your own problem. – JHBonarius May 23 '22 at 16:41
  • 1
    Instead of adding updates, strip it down. We can see the history if we need it, but we really only want to see what's relevant. – madreflection May 23 '22 at 16:42
  • @JHBonarius no one is complaining. – Mads May 23 '22 at 16:52
  • @madreflection strip down the existing question or close it and create new? – Mads May 23 '22 at 16:53
  • Strip down the current one. Posting a new one would be inappropriate. – madreflection May 23 '22 at 16:53
  • Thanks, will do so in a couple of hours. – Mads May 23 '22 at 17:08
  • 1
    In sql the line ending separator `;` is optional, but if you do use it, you can combine multiple commands in one execute. Still: drop the wrapper: Dapper itself already is the wrapper. What you are doing now is considered an anti pattern. – JHBonarius May 23 '22 at 18:12
  • @JHBonarius. I don't think so. The purpose is to simplify syntax further, and to decouple the user from the data access layer. It is called a "data access pattern". I am using this video as inspiration https://www.youtube.com/watch?v=dwMFg6uxQ0I&t=3125s&ab_channel=IAmTimCorey. – Mads May 23 '22 at 20:05
  • 1
    No it's not. You're missing the point of that video. You're supposed to build a data abstraction layer, not forward the execute method to the next layer and do the raw SQL/ADO there. That's not abstraction. That's making the design worse. – JHBonarius May 23 '22 at 20:25
  • @madreflection will you have a look at my updated question? – Mads May 24 '22 at 08:30

1 Answers1

0

I ended up creating a static class to hold the Sql-connection, since it can be reached from everywhere.

public static class SqlConnector
{
    public static IDbConnection SqlCon { get; set; }
}

Then I can rewrite the TableCreator class, to longer hold a Sql-connection like this:

public class TableCreator
{
    public void CreateTable()
    {
        SqlConnector.SqlCon.Execute("SELECT 5 AS Number INTO ##TempTable");
    }
}

And I can rewrite the main method like this:

TableCreator creator = new TableCreator();
DataInserter inserter = new DataInserter();

SqlConnector.SqlCon = new SqlConnection(connectionString);
using IDbConnection SqlCon = SqlConnector.SqlCon;
SqlConnector.SqlCon.Open();

creator.CreateTable();
inserter.InsertData();
Mads
  • 17
  • 1
  • 6