Is There A Better Way Of Writing This Stored Procedure?

Hi Experts,
I've just started learning to write Stored Procedure. The stored procedure listed below is working fine. I would like to know if there is anything that I should change to improve it?

The stored procedure will first construct a SQL string based on user input: Field1, Field2, Value1, Value2, Operator1, Condition1 and Condition2.

It then checks the total length of Value1 and Value2. If it equals 0, this means that this is the first time the page loads. So, it will execute the first SQL string. It length is more than 0 it will execute the second SQL string.

ALTER PROCEDURE dbo.test1
@Field1 varchar(50)='', /*Source = DropDownList1*/
@Field2 varchar(50)='', /*Source = DropDownList2*/
@Value1 varchar(50)='', /*Source = TextBox1*/
@Value2 varchar(50)='', /*Source = TextBox2*/
@Condition1 varchar(15)= ' OR ', /*Source = RadioButtons1*/
@Operator1 int=0, /*Source = DropDownList3*/
@Operator2 int=0, /*Source = DropDownList4*/
@startRowIndex varchar(5)='', /*Source = GridView1*/
@maximumRows varchar(5)='' /*Source = GridView1*/

AS
Begin
DECLARE @SqlLoad nvarchar(255)
DECLARE @SqlSearch nvarchar(2000)
DECLARE @Field1SQL varchar(255)
DECLARE @Field2SQL varchar(255)

      IF @Operator1=0
            BEGIN
            SET @Field1SQL=' (' + @Field1 + '  =''' + @Value1 + ''' OR ''' + @Value1 + ''' IS NULL) '
            END
      ELSE IF @Operator1=1
            BEGIN
            SET @Field1SQL=' ([' + @Field1 + '] LIKE ''%' + @Value1 + '%'') '
            END
      ELSE IF @Operator1=2
            BEGIN
            SET @Field1SQL=' ([' + @Field1 + '] NOT LIKE ''%' + @Value1 + '%'') '
      END

      IF @Operator2=0
            BEGIN
            SET @Field2SQL=' (' + @Field2 + '=''' + @Value2 + ''' OR ''' + @Value2 + ''' IS NULL)'
            END
      ELSE IF @Operator2=1
            BEGIN
            SET @Field2SQL=' ([' + @Field2 + '] LIKE ''%' + @Value2 + '%'') '
            END
      ELSE IF @Operator2=2
            BEGIN
            SET @Field2SQL=' ([' + @Field2 + '] NOT LIKE ''%' + @Value2 + '%'') '
      END

SET @SqlLoad = 'SELECT * FROM (SELECT *, ROW_NUMBER() OVER (ORDER BY FirstName) AS RowRank
 FROM Authors) As AuthorsWithRowNumbers
WHERE (RowRank >' + @startRowIndex + ') AND (RowRank <= (' + @startRowIndex + '+' + @maximumRows +'))'

SET @SqlSearch = 'SELECT * FROM (SELECT *, ROW_NUMBER() OVER (ORDER BY FirstName) AS RowRank
 FROM Authors WHERE ('  + @Field1SQL + @Condition1 + @Field2SQL + ')) As AuthorsWithRowNumbers
WHERE (RowRank >' + @startRowIndex + ') AND (RowRank <= (' + @startRowIndex + '+' + @maximumRows +'))'

IF (LEN(@Value1) + LEN(@Value2))=0
      EXEC sp_executesql @SqlLoad
ELSE
      EXEC sp_executesql @SqlSearch
END

Thanks
noobe1Asked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

dqmqCommented:
Perhaps some cosmetics:

ALTER PROCEDURE dbo.test1
@Field1 varchar(50)='', /*Source = DropDownList1*/
@Field2 varchar(50)='', /*Source = DropDownList2*/
@Value1 varchar(50)='', /*Source = TextBox1*/
@Value2 varchar(50)='', /*Source = TextBox2*/
@Condition1 varchar(15)= ' OR ', /*Source = RadioButtons1*/
@Operator1 int=0, /*Source = DropDownList3*/
@Operator2 int=0, /*Source = DropDownList4*/
@startRowIndex varchar(5)='', /*Source = GridView1*/
@maximumRows varchar(5)='' /*Source = GridView1*/
 
AS
Begin
DECLARE @SqlLoad nvarchar(2000)
DECLARE @SqlSearch nvarchar(2000)
DECLARE @Field1SQL varchar(255)
DECLARE @Field2SQL varchar(255)
 
      IF @Operator1=0
            BEGIN
            SET @Field1SQL=' (' + @Field1 + '  =''' + @Value1 + ''' OR ''' + @Value1 + ''' IS NULL) '
            END
      ELSE IF @Operator1=1
            BEGIN
            SET @Field1SQL=' ([' + @Field1 + '] LIKE ''%' + @Value1 + '%'') '
            END
      ELSE IF @Operator1=2
            BEGIN
            SET @Field1SQL=' ([' + @Field1 + '] NOT LIKE ''%' + @Value1 + '%'') '
      END
 
      IF @Operator2=0
            BEGIN
            SET @Field2SQL=' (' + @Field2 + '=''' + @Value2 + ''' OR ''' + @Value2 + ''' IS NULL)' 
            END
      ELSE IF @Operator2=1
            BEGIN
            SET @Field2SQL=' ([' + @Field2 + '] LIKE ''%' + @Value2 + '%'') '
            END
      ELSE IF @Operator2=2
            BEGIN
            SET @Field2SQL=' ([' + @Field2 + '] NOT LIKE ''%' + @Value2 + '%'') '
      END
 
IF (LEN(@Value1) + LEN(@Value2))<>0
   SET @SqlSearch = 'WHERE ('  + @Field1SQL + @Condition1 + @Field2SQL + ')'
ELSE 
   SET @SqlSearch= = 'WHERE 1=1'
 
SET @SqlLoad =
'SELECT * FROM ' +
' (SELECT *, ROW_NUMBER() OVER (ORDER BY FirstName) AS RowRank ' +
'    FROM Authors ' + @SqlSearch + 
' ) As AuthorsWithRowNumbers ' +
'WHERE (RowRank >' + @startRowIndex + ') ' +
'  AND (RowRank <= (' + @startRowIndex + '+' + @maximumRows +')) '
 
EXEC sp_executesql @SqlLoad
 
END

Open in new window

0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
ee_rleeCommented:
hi

also put [] here
' (' + @Field1 + '  ='''

this statement
 ''' OR ''' + @Value1 + ''' IS NULL) '
will not work because '' is not = null, so it will always return false
compare it to '' or check if len = 0  if you want to check if @Value is empty
0
Guy Hengel [angelIII / a3]Billing EngineerCommented:
as from sql 2005, you have the option to avoid one of the big problems with the dynamic sql, ie that the dynamic sql is executed, by default, under the permissions of the caller and NOT of the procedure owner...

read up on the EXECUTE AS SELF (vs EXECUTE AS CALLER) to be added to the procedure header  
0
brejkCommented:
I would also suggest putting SET NOCOUNT ON in the beginning of the procedure to avoid sending useless messages to the applications.
0
noobe1Author Commented:
Thanks everyone for the comments. Points will be distributed evenly.
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Microsoft SQL Server 2005

From novice to tech pro — start learning today.