Database connection class performance

Question

This is my general database connection class. I am using this class to execute my queries through website.

What would your suggestions on how to improve performance be?

(Running MSSQL 2008 R2 SP1 - Microsoft Visual Studio 2010 SP1 , C# 4.0 - ASP.net 4.0)

Class:

using System;
using System.Collections.Generic;
using System.Collections;
using System.Linq;
using System.Web;
using System.Data.Sql;
using System.Data.SqlClient;
using System.Data;
using System.IO;
/// <summary>
/// Summary description for DbConnection
/// </summary>
public class DbConnection
{
 public static string srConnectionString = "server=localhost;database=myDB;uid=sa;pwd=MYPW;";
 public DbConnection()
 {
 }
 public static DataSet db_Select_Query(string strQuery)
 {
 DataSet dSet = new DataSet();
 try
 {
 using (SqlConnection connection = new SqlConnection(srConnectionString))
 {
 connection.Open();
 SqlDataAdapter DA = new SqlDataAdapter(strQuery, connection);
 DA.Fill(dSet);
 }
 return dSet;
 }
 catch (Exception)
 {
 using (SqlConnection connection = new SqlConnection(srConnectionString))
 {
 if (srConnectionString.IndexOf("select Id from tblAspErrors") != -1)
 {
 connection.Open();
 strQuery = strQuery.Replace("'", "''");
 SqlCommand command = new SqlCommand("insert into tblSqlErrors values ('" + strQuery + "')", connection);
 command.ExecuteNonQuery();
 }
 }
 return dSet;
 }
 }
 public static void db_Update_Delete_Query(string strQuery)
 {
 try
 {
 using (SqlConnection connection = new SqlConnection(srConnectionString))
 {
 connection.Open();
 SqlCommand command = new SqlCommand(strQuery, connection);
 command.ExecuteNonQuery();
 }
 }
 catch (Exception)
 {
 strQuery = strQuery.Replace("'", "''");
 using (SqlConnection connection = new SqlConnection(srConnectionString))
 {
 connection.Open();
 SqlCommand command = new SqlCommand("insert into tblSqlErrors values ('" + strQuery + "')", connection);
 command.ExecuteNonQuery();
 }
 }
 }
}

• Jesse C. Slicer: SqlCommand implements IDisposable, so those also should be wrapped in using (similar to what you're correctly doing with SqlConnection).

Answer 1

You are not using a DataContext, so presuming you don't want to for this, I'd suggest that you also consider a connection pool, that way you don't need to incur the connection cost for every select/delete.

Additionally, your catches are very heavy, are you getting that many errors that you need to track? You may want to see this question as an alternative to logging the errors yourself: https://stackoverflow.com/questions/5076303/sql-server-error-messages

@kubal5003 - good point on automatic connection pooling, here is the MSDN that discusses it and its sibling that lists the connection string properties and defaults

• Furkan Gözükara: hello thanks for answers. actually i almost never get any error. this connection pool i am really interested in. can you provide me any good article tutorial about this.
• kubal5003: ADO.NET already uses connection pooling without the user knowing about it, so you don't have to do anything in here.
• Furkan Gözükara: ye i also asked on official forum and they said the latest version is fine. here the link : forums.asp.net/p/1723810/4612236.aspx/…

Answer 2

I don't have a comment on your performance problem, but I wanted to comment on this...

public static void db_Update_Delete_Query(string strQuery)
{
 try
 {
 using (SqlConnection connection = new SqlConnection(srConnectionString))
 {
 connection.Open();
 SqlCommand command = new SqlCommand(strQuery, connection);
 command.ExecuteNonQuery();
 }
 }
 catch (Exception)
 {
 strQuery = strQuery.Replace("'", "''");
 using (SqlConnection connection = new SqlConnection(srConnectionString))
 {
 connection.Open();
 SqlCommand command = new SqlCommand("insert into tblSqlErrors values ('" + strQuery + "')", connection);
 command.ExecuteNonQuery();
 }
 }
}

First, DB calls in a catch block is probably a really bad idea. I'm not sure why you think the second call will work when the first call has failed, it makes absolutely no sense.

As for your question, you need to include more information. I am not an ADO expert by any means, so I'm not going to guess as to where your problem lies, but you at least need to provide us with:

1. Your performance requirements.
2. Your benchmark results (you have run benchmark tests, right?)
3. Your usage scenario.

• radarbob: +1. This corrupts and thwarts the whole reason for try/catch. If "try/catching" the first DB call is a good idea, why not that second.. And if that second "try/catch" is a good idea .... You get the idea.
• Furkan Gözükara: try catch perfectly works. for example you are trying to insert data to non existing table. the catch block will catch and store it .)
• Ed Swangren: @MonsterMMORPG: The problem is catch(Exception). Anything could have gone wrong there. If that were the case then he should be catching InvalidOperationException instead. What if the error arose because the DB was unreachable? What if the connection string was bad?
• Mark Brackett: @EdS. - I generally agree, but feel its worth noting that in most exceptional cases the 2nd db call will fail in the same way as the first. So its pretty much a wash - you'll still get the exception info, but will have lost some call stack and possible info from swallowing the first exception. Overall, it's a little less than well thought out - but I don't see it as a huge deal. FWIW.

Answer 3

Sql Injection in your catch clause - just use a parameter for the strQuery value instead of trying to escape it.

That said, I don't see any performance problems with this - it's basically raw ADO.NET, which is more or less the fastest you'll get.

DataSets are a little heavy compared to DataReaders, but it's not likely to be the source of any performance problems.

The main performance issue with any database is going to be the queries you send to it - not really the DB access code.