How can I improve upon this "pattern"?

Question

I have a series of method calls that all share similar qualities. The basic layout is

public void CallDataBase()
{
   Utility sqlHelper = new Utility();   

   StringBuilder query = new StringBuilder(200);
   query.Append(@"update " + Environment.NewLine);//or insert, select
   query.Append(@" table " + Environment.NewLine);
   //...
   utility.ConnectAndExecuteNonQuery(query);//or query, scalar  
}

I'm not a big fan of the StringBuilder in this case, but it does help readability to have line breaks in the inline SQL.

Stored Procedures are not an option (way too much work to convert, have higher priorities). LINQ and EF are not options. Company is on SQL 2000 and there are no concrete plans to upgrade.

Answer 1

I recently refactored a bunch of our database code to use the Dapper ORM. This resulted in code that looks like this.

using (var connection = new SqlConnection(connectionString))
{
    connection.Open();

    var p = new DynamicParameters();

    p.Add("@ServerIP", "192.168.100.200");
    p.Add("@PartName", "Dapper");
    p.Add("@Config_Data_Modified", false);
    p.Add("@LatestRecord", false);
    p.Add("@IsRunning", true);
    p.Add("@Stopped", false);
    p.Add("@SessionID", dbType: DbType.Int32, direction: ParameterDirection.Output);

    connection.Execute(
        "DECLARE @TmpTable TABLE (ID INT) " +
        "INSERT Session(ServerIP, PartName, Config_Data_Modified, LatestRecord, IsRunning, Stopped) " +
        "OUTPUT Inserted.SessionID INTO @TmpTable " +
        "VALUES (@ServerIP, @PartName, @Config_Data_Modified, @LatestRecord, @IsRunning, @Stopped)" +
        "SELECT @SessionID = ID FROM @TmpTable", p);

    var sessionID = p.Get<Int32>("@SessionID");
}

This improved the readability of the code tremendously in my opinion. I don't know what all is going on in your Utility class, but if it is just a simple DAL, then it shouldn't be too hard to replace it with a lightweight ORM such as Dapper.

Answer 2

Leaving aside the SQL issues (use parameterised procs etc instead of inline SQL), and addressing the code in the question.

Instead of using string builder, use System.IO.StringWriter it uses a StringBuilder under the covers but gives you the full TextWriter interface we're familiar with from System.Console and System.IO.StreamWriter

It makes things far more readable IMHO

public void CallDataBase()
{
   Utility sqlHelper = new Utility();   

   StringWriter query = new StringWriter();
   query.WriteLine("update {0}", tableName);//or insert, select
   query.WriteLine(" set " );
   //...
   utility.ConnectAndExecuteNonQuery(query.ToString());//or query, scalar   
} 

Also, you can construct it with a StringBuilder instance if you really need to initialise the StringBuilder.

   StringWriter query = new StringWriter(new StringBuilder(200));

Answer 3

I find SQL within C# code rather horrifying. It looks inelegant with all those string concatenations, is hard to modify, hard to reuse queries, and hard to test because you can't just copy the query from the source code and paste it into a database tool like SSMS.

I prefer to put all SQL in one or more resource files (Add --> Resource File). I usually name it "sql.resx".

Rather than:

connection.Execute(
    "DECLARE @TmpTable TABLE (ID INT) " +
    "INSERT Session(ServerIP, PartName, Config_Data_Modified, LatestRecord, IsRunning, Stopped) " +
    "OUTPUT Inserted.SessionID INTO @TmpTable " +
    "VALUES (@ServerIP, @PartName, @Config_Data_Modified, @LatestRecord, @IsRunning, @Stopped)" +
    "SELECT @SessionID = ID FROM @TmpTable", p);

I like code to look more like:

connection.Execute(sql.InsertStuffIntoSomeTable, p);

With the resource file storing the copyable, editable, reusable, testable SQL text without all that + "junk" +.

    DECLARE @TmpTable TABLE (ID INT)
    INSERT Session(ServerIP, PartName, Config_Data_Modified, LatestRecord, IsRunning, Stopped)
    OUTPUT Inserted.SessionID INTO @TmpTable
    VALUES (@ServerIP, @PartName, @Config_Data_Modified, @LatestRecord, @IsRunning, @Stopped)
    SELECT @SessionID = ID FROM @TmpTable

Answer 4

Disclaimer: I have no experience in the C# world, my observations are entirely based on the Java space.

I have seen fluent APIs being used for this. With some of the sophisticated ones, you'll be able to write (typed or untyped, depending on the API) at least somewhat LINQ-like queries.

Anything will feel like a compromise though, as any language flexible enough to handle complex queries will tend to become typing-intensive.

Answer 5

How is it too much work to create a stored procedure?

If you're willing to go in and modify the code anyhow, how would it be any more work to copy the inline sql into a proc?

At the very least, use parametrized queries. You could store the SQL for them in sql files, and load them as needed (and cache them as they are loaded)?

That way you can simply write the sql without all the extra c# markup.

Answer 6

I have created one class that helps me with executing and loading SQL Query results, so this is class

public static VOSqlResult Load(string cName, string command, params SqlParameter[] parameters)
        {
            DataTable tbl = new DataTable();
            VOSqlResult result = InitResult();
            SqlConnection cn = CreateConnection(cName);
            try
            {
                cn.Open();
                SqlCommand cmd = new SqlCommand(command);
                cmd.CommandTimeout = 3600;
                foreach (SqlParameter parameter in parameters)
                {
                    cmd.Parameters.Add(parameter);
                }
                cmd.Connection = cn;
                cmd.CommandType = CommandType.Text;
                SqlDataReader reader = cmd.ExecuteReader();
                if (reader != null)
                    result.Table.Load(reader);
            }
            catch (Exception ex)
            {
                Logger.Error(String.Format("DataHelper -> Loading data [{1}] {0}", ex.Message, command), ex);
                result.Error = ex.Message;
                result.Valid = false;
            }
            finally
            {
                CloseConnection(cn);
            }
            return result;
        }

        public static VOSqlResult ExecuteScalar(string cName, string command, params SqlParameter[] parameters)
        {
            DataTable tbl = new DataTable();
            VOSqlResult result = InitResult();
            SqlConnection cn = CreateConnection(cName);
            try
            {
                cn.Open();

                SqlCommand cmd = new SqlCommand(command);
                cmd.CommandTimeout = 3600;
                foreach (SqlParameter parameter in parameters)
                {
                    cmd.Parameters.Add(parameter);
                }
                cmd.Connection = cn;
                cmd.CommandType = CommandType.Text;
                cmd.ExecuteScalar();
            }
            catch (Exception ex)
            {
                Logger.Error(String.Format("DataHelper -> Executing scalar [{1}] {0}", ex.Message, command), ex);
                result.Error = ex.Message;
                result.Valid = false;
            }
            finally
            {
                CloseConnection(cn);
            }
            return result;
        }

        private static void CloseConnection(SqlConnection cn)
        {
            try
            {
                cn.Close();
            }
            catch (Exception ex)
            {
                Logger.Error(String.Format("DataHelper Closing COnnection -> {0}", ex.Message), ex);
                //TODO: create more dynamic solution
            }
        }

        private static VOSqlResult InitResult()
        {
            VOSqlResult v = new VOSqlResult();
            v.Table = new DataTable();
            v.Valid = true;
            v.Error = "";
            return v;
        }

        private static SqlConnection CreateConnection(string cName)
        {
            SqlConnection cn = new SqlConnection(ConfigReader.CStringGet(cName));

            return cn;
        }

        public static VOSqlResult ExecuteStoreProcedure(string cName, string spName, params SqlParameter[] parameters)
        {
            DataTable tbl = new DataTable();
            VOSqlResult result = InitResult();
            SqlConnection cn = CreateConnection(cName);
            try
            {
                cn.Open();
                SqlCommand cmd = new SqlCommand(spName);
                cmd.CommandTimeout = 3600;
                foreach (SqlParameter parameter in parameters)
                {
                    cmd.Parameters.Add(parameter);
                }
                cmd.Connection = cn;
                cmd.CommandType = CommandType.StoredProcedure;
                SqlDataReader reader = cmd.ExecuteReader();
                if (reader != null)
                    result.Table.Load(reader);
            }
            catch (Exception ex)
            {
                StringBuilder sb = new StringBuilder();
                foreach (var sqlParameter in parameters)
                {
                    sb.AppendFormat("[{0} -> {1}]", sqlParameter.ParameterName, sqlParameter.Value.ToString());
                }

                Logger.Error(String.Format("DataHelper -> Executing store procedure {1} with the params {2} -- {0}", ex.Message, spName, sb.ToString()), ex);
                result.Error = ex.Message;
                result.Valid = false;
            }
            finally
            {
                CloseConnection(cn);
            }

            return result;
        }
    }

    public class VOSqlResult
    {
        public String Error;
        public DataTable Table;
        public Boolean Valid;        
    }

I have one more class ConfigReader that can read connection strings but you can replace it with ConfigurationManager

and this is how to use it

VOSqlResult rez = DataHelper.Load("connection string name", @"Select * from 
                      my_table where
                         Email = @Email", new SqlParameter("@Email", "[email protected]"));

you can have as many parameters as you wish. Simple enough for