Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

###Anonymized code

Anonymized code

Object/variable names should tell us something about what they actually mean. For instance, these variables:

 DECLARE @val1 NVARCHAR(10) 
 DECLARE @val2 INT

What does val1 and val2 actually stand for? In fact everything in the query looks like example code, so it leaves us to guess what the code is actually designed for.


###SELECT INTO #table

SELECT INTO #table

You would most likely get performance improvements with regards to that temp table if you created it manually instead of using SELECT ... INTO. That would at least let you create some indexes on it. I've gained huge performance improvements with that technique in the past so I strongly recommend explicitly creating your temp tables along with indexes for performance-critical queries.


###Cursor

Cursor

You would likely get better performance in row-by-row operations if you used a cursor, instead of doing an update both to the source table and the temp table in a while loop. See cursors on MSDN for reference.


###Print @msg

Print @msg

I'm not really sure why you would print the @msg variable. According to your question, the purpose of the code is:

The code attempts to change the value of test.[stat] to 'R' if a subset is found with the corresponding stat values.

So printing that message to the console seems like a waste of resources. I notice you are returning @msg as output anyways, so if the calling code needs that, it's easy to get it without printing it to the console.

###Anonymized code

Object/variable names should tell us something about what they actually mean. For instance, these variables:

 DECLARE @val1 NVARCHAR(10) 
 DECLARE @val2 INT

What does val1 and val2 actually stand for? In fact everything in the query looks like example code, so it leaves us to guess what the code is actually designed for.


###SELECT INTO #table

You would most likely get performance improvements with regards to that temp table if you created it manually instead of using SELECT ... INTO. That would at least let you create some indexes on it. I've gained huge performance improvements with that technique in the past so I strongly recommend explicitly creating your temp tables along with indexes for performance-critical queries.


###Cursor

You would likely get better performance in row-by-row operations if you used a cursor, instead of doing an update both to the source table and the temp table in a while loop. See cursors on MSDN for reference.


###Print @msg

I'm not really sure why you would print the @msg variable. According to your question, the purpose of the code is:

The code attempts to change the value of test.[stat] to 'R' if a subset is found with the corresponding stat values.

So printing that message to the console seems like a waste of resources. I notice you are returning @msg as output anyways, so if the calling code needs that, it's easy to get it without printing it to the console.

Anonymized code

Object/variable names should tell us something about what they actually mean. For instance, these variables:

 DECLARE @val1 NVARCHAR(10) 
 DECLARE @val2 INT

What does val1 and val2 actually stand for? In fact everything in the query looks like example code, so it leaves us to guess what the code is actually designed for.


SELECT INTO #table

You would most likely get performance improvements with regards to that temp table if you created it manually instead of using SELECT ... INTO. That would at least let you create some indexes on it. I've gained huge performance improvements with that technique in the past so I strongly recommend explicitly creating your temp tables along with indexes for performance-critical queries.


Cursor

You would likely get better performance in row-by-row operations if you used a cursor, instead of doing an update both to the source table and the temp table in a while loop. See cursors on MSDN for reference.


Print @msg

I'm not really sure why you would print the @msg variable. According to your question, the purpose of the code is:

The code attempts to change the value of test.[stat] to 'R' if a subset is found with the corresponding stat values.

So printing that message to the console seems like a waste of resources. I notice you are returning @msg as output anyways, so if the calling code needs that, it's easy to get it without printing it to the console.

Source Link
Phrancis
  • 20.5k
  • 6
  • 69
  • 155

###Anonymized code

Object/variable names should tell us something about what they actually mean. For instance, these variables:

 DECLARE @val1 NVARCHAR(10) 
 DECLARE @val2 INT

What does val1 and val2 actually stand for? In fact everything in the query looks like example code, so it leaves us to guess what the code is actually designed for.


###SELECT INTO #table

You would most likely get performance improvements with regards to that temp table if you created it manually instead of using SELECT ... INTO. That would at least let you create some indexes on it. I've gained huge performance improvements with that technique in the past so I strongly recommend explicitly creating your temp tables along with indexes for performance-critical queries.


###Cursor

You would likely get better performance in row-by-row operations if you used a cursor, instead of doing an update both to the source table and the temp table in a while loop. See cursors on MSDN for reference.


###Print @msg

I'm not really sure why you would print the @msg variable. According to your question, the purpose of the code is:

The code attempts to change the value of test.[stat] to 'R' if a subset is found with the corresponding stat values.

So printing that message to the console seems like a waste of resources. I notice you are returning @msg as output anyways, so if the calling code needs that, it's easy to get it without printing it to the console.

lang-sql

AltStyle によって変換されたページ (->オリジナル) /