Proper parameterized query class for asp.net - c# application - sql server

Question

I am starting to encapsulate my queries in a single parameterized query class

Now can you please evaluate my classes for performance security and every other aspect possible ?

Do you have any suggestions ?

Here my select class

public static DataSet cmd_SelectQuery(string srCommandText, List<string> lstParameterNames, List<string> lstParameters)
{
    DataSet dsCmdPara = new DataSet();
    try
    {
        using (SqlConnection connection = new SqlConnection(DbConnection.srConnectionString))
        {
            using (SqlCommand cmd = new SqlCommand(srCommandText, connection))
            {
                cmd.CommandType = CommandType.Text;
                for (int i = 0; i < lstParameterNames.Count; i++)
                {
                    cmd.Parameters.AddWithValue(lstParameterNames[i], lstParameters[i]);
                }
                connection.Open();
                using (SqlDataAdapter sqlDa = new SqlDataAdapter(cmd))
                {
                    sqlDa.Fill(dsCmdPara);
                    return dsCmdPara;
                }
            }
        }
    }
    catch (Exception E)
    {
        csPublicFunctions.insertIntoTblSqlErrors(srCommandText + " " + E.Message.ToString());
    }
    return dsCmdPara;
}

And here my update,delete class

public static void cmd_UpdateDeleteQuery(string srCommandText, List<string> lstParameterNames, List<string> lstParameters)
{
    try
    {
        using (SqlConnection connection = new SqlConnection(DbConnection.srConnectionString))
        {
            using (SqlCommand cmd = new SqlCommand(srCommandText, connection))
            {
                cmd.CommandType = CommandType.Text;
                for (int i = 0; i < lstParameterNames.Count; i++)
                {
                    cmd.Parameters.AddWithValue(lstParameterNames[i], lstParameters[i]);
                }
                connection.Open();
                cmd.ExecuteNonQuery();
            }
        }
    }
    catch (Exception E)
    {
        csPublicFunctions.insertIntoTblSqlErrors(srCommandText + " " + E.Message.ToString());
    }
}

Thanks a lot for answers

What if one of your parameters is not a string? (for example select * from table where ID = @tableID and ID is a numeric field) — Steve
– Steve, Commented Dec 24, 2012 at 21:04
Well obviously i can not pass :D You can post your suggestion class as answer — Furkan Gözükara
– Furkan Gözükara, Commented Dec 24, 2012 at 21:05
Make your paramter IList<object> lstParameters or simply IList lstParameters. IList allows you to pass lists as well as arrays and object allows you to pass the parameters in the correct type. — Olivier Jacot-Descombes
– Olivier Jacot-Descombes, Commented Dec 24, 2012 at 21:08
@Steve that does not causes any problem as long as the parameter i gave class is integer. — Furkan Gözükara
– Furkan Gözükara, Commented Dec 24, 2012 at 21:08

Giovanni Galbo · Accepted Answer · 2012-12-24 21:14:36Z

2

Two quick suggestions since I'm writing this on a phone.

Take in objects, not string
Taking in params object[] as the parameter list might make your methods a little more natural to use.

answered Dec 24, 2012 at 21:14

Giovanni Galbo

13.1k14 gold badges63 silver badges80 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Furkan Gözükara Over a year ago

thanks but what do you mean by "params object[] as the parameter list might make your methods a little more natural to use." do you mean each object should be a list of objects ?

Giovanni Galbo Over a year ago

Params lets you do something like this: Query("select * from order where id = {0} and status = {1}", id, 5); Notice I didn't have to new up a list to put the params inside.

Erick T · Accepted Answer · 2012-12-27 20:43:06Z

1

The code is valid and correct as it stands. You asked for feedback, so I have the following points.

(1) In the code where you are building up the parameters, it's a good idea to specify the data type.

for (int i = 0; i < lstParameterNames.Count; i++)
{
    var parameter = cmd.Parameters.AddWithValue(lstParameterNames[i], lstParameters[i]);
    parameter.SqlDbType = SqlDbType.NVarChar; // or whatever type you need
}

(2) You don't need to explicitly open/close the connection, as the Fill method does that for you.

connection.Open();
using (SqlDataAdapter sqlDa = new SqlDataAdapter(cmd))
{
    sqlDa.Fill(dsCmdPara);
    connection.Close();
    return dsCmdPara;
}

(3) If you are only going to have one table, use a DataTable instead of a DataSet. It will be faster and less complex.

(4) This is more of a style point, but I find it is very helpful to explicitly close your connections. If there is an exception in the closing, having the line number (via the pdb) is very helpful. Also, it helps make sure you don't leak connections if you forget a using.

Edit

I want to clarify point 4. Assume that the when a connection in closed, the cleanup caused an exception. If you don't explicitly close the connection, the exception would be raised in the Dispose method, which might be doing a bunch of other work. This makes it harder to see what I did to cause the problem.

Take this simple example. There is an underlying resource that needs to be cleaned up (the entire reason for Dispose), and it throws an exception.

public class Resource: IDisposable
{
    public void Close()
    {
        CleanupMemory();
    }

    private void CleanupMemory()
    {
        throw new Exception();
    }

    public void Dispose()
    {
        CleanupMemory();
    }
}

The (cleaned up) stack trace for the two different approaches are:

using (var r = new Resource())
{
    r.Close();
}

   at ConsoleApplication1.Resource.CleanupMemory() in Program.cs:line 31
   at ConsoleApplication1.Resource.Close() in Program.cs:line 26
   at ConsoleApplication1.Program.Main(String[] args) Program.cs:line 16

versus

using (var r = new Resource())
{

}


   at ConsoleApplication1.Resource.CleanupMemory() in Program.cs:line 35
   at ConsoleApplication1.Resource.Dispose() in Program.cs:line 40
   at ConsoleApplication1.Program.Main(String[] args) in Program.cs:line 18

In the first example, I know that my calling Close caused the exception, I have a place to start debugging. If I only know that the exception was thrown, I need to completely rely on the information in the exception. I have no other context. And most exception messages are not especially clear. :)

Both are totally correct, just different.

Erick

edited Dec 27, 2012 at 20:43

answered Dec 24, 2012 at 21:29

Erick T

7,48911 gold badges55 silver badges90 bronze badges

4 Comments

Furkan Gözükara Over a year ago

thanks a lot for suggestions. as for stands assume that an error happened inside using. wouldn't it close connection ? Also it seems like my system is wasted a lot with datasets since i built it that way ^^ even though almost 99% time i select only 1 table

Erick T Over a year ago

It's not so much that the connection won't be closed, but that any exception would be raised from inside the code generated by the using block. When this happens, you don't have access to your local variables, making debugging more difficult.

Furkan Gözükara Over a year ago

what do you mean by "It's not so much that the connection won't be closed"

Erick T Over a year ago

If there are using blocks for every disposable object, then the resources will be cleaned up. This includes closing the connection.

Musa · Accepted Answer · 2014-12-25 16:17:53Z

public DataTable Get_DTable(String Query, Dictionary<String, String> Parameters)
    {
        try
        {
            using (con = new SqlConnection(cls_Connection.URL()))
            {
                if (con.State == 0)
                    con.Open();
                using (cmd = new SqlCommand(Query, con))
                {
                    foreach (KeyValuePair<string, string> item in Parameters)
                    {
                        cmd.Parameters.AddWithValue(item.Key, item.Value);
                    }

                    using (da = new SqlDataAdapter(cmd))
                    {
                        using (dt = new DataTable())
                        {
                            da.Fill(dt);
                            if (dt.Rows.Count > 0)
                            {
                                return dt;
                            }
                        }
                    }
                }
            }
        }
        catch (Exception exp)
        {
            MessageBox.Show(exp.Message,
                            "Information",
                            MessageBoxButtons.OK,
                            MessageBoxIcon.Information);
        }
        return null;
    }

Collectives™ on Stack Overflow

Proper parameterized query class for asp.net - c# application - sql server

3 Answers 3

2 Comments

4 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

4 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related