Is it ok to use dynamic SQL in this case?

Let me start by saying I am biase开发者_如何学God; I hate dynamic SQL under all circumstances. That being said, is this scenario considered good practice for dynamic SQL?

sqlDataSourceObject.SelectCommand = String.Concat(
                    "select top ", maxRows,
                    "   col1, ",
                    "   col2 as myData, ",
                    "   '' as blah, ",
                    "   col3 as Fromperson ",
                    "   'Corporate' as toPerson, ",
                    "   Convert(char(11), orderDate) as orderDate, ",
                    "   carrier, ",
                    sqlString1,
                    sqlString2,
                    sqlString3 + " AND areaCode = '" + currArea + "'"
                    );

This query may run once, then change the value for sqlString1,2,3, or currArea and run it again against a different SqlDataSource.

This code makes me angry to read. Its hard to read, it can change with the sqlString variables, I cant run it without copy/pasting into SSMS and I have to go track down several variables to make a single change.

But, like I said I am biased so I am asking you. Is this code, written in 2001 before LINQ, as good as a stored proc or some other technology, generally OK from a good practice perspective?

If not, how would you have improved it (remember no LINQ, this is 2001).

A point of clarification:

Dynamic SQL is normally taken to mean that the semantics of the statement change based on some external factor. In other words, the column names or even the base table(s) might be altered. This was common to do for pivot queries in the old days.

It's kind of hard to tell because I don't know what's going into those awfully-named sqlStringX parameters, but I think that what I'm seeing here is really just inline SQL which happens to be riddled with SQL injection vulnerabilities. It is trivially easy to parameterize. Fix this ASAP, please. Inline SQL is fine but there is no reason to be using raw strings instead of parameters.

Stored procedures would be one idea of how to better handle these types of queries. Granted the stored proc may just execute what the parameters pass but that would be my suggestion for one way to improve that code so that the DBA can know what indexes may be useful to help optimize the query. SQL injection attacks as @Jarrod Roberson points out are also quite likely with this kind of code.

PS: I wrote this kind of code back in 1998 where I had ~20 possible parameters in writing a "Find Customer" routine that was one of my first assignments out of university so I do understand where this kind of code can originate.

I'd use a stored procedure myself. But in any case, no matter what, use parameters. They way you' have it there is not secure at all, and as you say, makes me angry to look at. :-)

Here's one reference that might help (not stored procs per se, but still uses parms) http://www.asp.net/data-access/tutorials/using-parameterized-queries-with-the-sqldatasource-vb