3

I'm using parametrized queries in a project to prevent SQL injection and I ran into an interesting scenario with query. I have a query that will some times have more parameters than others i.e. the where clause changes. Is there any difference, in performance or otherwise, between the two following code blocks? This code is inside an object so the "variables" are properties and both methods have access.

In this one I only add the parameters if the condition is met.

    public bool TestQuery()
    {
        SqlCommand command = new SqlCommand();
        string query = GetQuery(command);
        command.CommandText = query;
        //execute query and other stuff
    }
    private string GetQuery(SqlCommand command  )
    {
        StringBuilder sb = new StringBuilder("SELECT * FROM SomeTable WHERE Active = 1 ");
        if (idVariable != null)
        {
            sb.Append("AND id = @Id");
            command.Parameters.Add("@Id", SqlDbType.Int).Value = idVariable;
        }
        if (!string.IsNullOrEmpty(colorVariable))
        {
            sb.Append("AND Color = @Color");
            command.Parameters.Add("@Color", SqlDbType.NVarChar).Value = colorVariable;
        }
        if (!string.IsNullOrEmpty(sizeVariable))
        {
            sb.Append("AND Color = @Size");
            command.Parameters.Add("@Size", SqlDbType.NVarChar).Value = sizeVariable;
        }
        return sb.ToString();
    }

In this one I add all of the parameters every time and only add the where clause arguments if the condition is met.

    public bool TestQuery()
    {
        SqlCommand command = new SqlCommand(GetQuery());
        command.Parameters.Add("@Id", SqlDbType.Int).Value = idVariable;
        command.Parameters.Add("@Color", SqlDbType.NVarChar).Value = colorVariable;
        command.Parameters.Add("@Size", SqlDbType.NVarChar).Value = sizeVariable;
        //execute query and other stuff
    }
    private string GetQuery()
    {
        StringBuilder sb = new StringBuilder("SELECT * FROM SomeTable WHERE Active = 1 ");
        if (idVariable != null)
            sb.Append("AND id = @Id");
        if (!string.IsNullOrEmpty(colorVariable))
            sb.Append("AND Color = @Color");
        if (!string.IsNullOrEmpty(sizeVariable))
            sb.Append("AND Color = @Size");
        return sb.ToString();
    }

According to a test I did either of them will work. I personally prefer the second one because I feel it's cleaner and easier to read but I wonder if there is some performance/security reason I shouldn't be adding parameters that aren't used and are probably going to be null/empty string.

1
  • The first choice would be my preference. It keeps the parameters together with the code that uses them and ought to make it more difficult to reference parameters unintentionally, e.g. when someone is doing maintenance. It also avoids any question of how to populate parameters for which you may not have values (or values are expensive to obtain), but won't be using in the query. Generally speaking, performance and security shouldn't suffer either way. If the code was modified to allow SQL injection a clever (ab)user could access any supplied parameters, even if you don't use them. Commented Feb 15, 2013 at 20:59

1 Answer 1

1

I guess I'll go with option one per HABO's comment then since beargle's answer doesn't really work in my situation..

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

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.