March 26, 2008 at 2:44 pm
Is there a consensus out there?
Developer one codes the loop like this:
[font="Courier New"]DECLARE tblcur CURSOR STATIC LOCAL FOR (your select statement)
OPEN tblcur
WHILE 1 = 1
BEGIN
FETCH tblcur INTO (field list)
IF @@fetch_status <> 0
BREAK
(do whatever you need to do)
END
DEALLOCATE tblcur
[/font]
Developer two thinks this is better:
[font="Courier New"]DECLARE tblcur CURSOR STATIC LOCAL FOR (your select statement)
OPEN tblcur
FETCH tblcur INTO (field list)
WHILE (@@fetch_status = 0)
BEGIN
(do whatever you need to do)
FETCH tblcur INTO (field list)
END
DEALLOCATE tblcur[/font]
Points to consider in your opinion:
1. Developer one claims that method one is easier to maintain because the fetch statement occurs only once - i.e. if list of fields changes, the change need only be written in one place.
2. Developer two claims that method one is wrong because WHILE conditional is always true and therefore it is not a legal WHILE loop.
Thanks for your feedback.
Chris Judge
March 26, 2008 at 2:48 pm
In answer to your question, I would say that it's nothing more than a matter of preference. If either of the developers are telling you that theirs is "faster" or "better" than the other, just ask them to prove it.
On the other hand, challenge your developers to not use a cursor in the first place, then see what they say. 😀
______________________________________________________________________
Personal Motto: Why push the envelope when you can just open it?
If you follow the direction given HERE[/url] you'll likely increase the number and quality of responses you get to your question.
Jason L. SelburgMarch 27, 2008 at 6:23 am
I agree that NOT using a cursor is an even better approach.
However, to answer your question, I wouldn't use the WHILE loop with a BREAK statement because it's really a violation of logic.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
March 27, 2008 at 6:34 am
I personally would use the second option - if someone was holding a gun to my head telling me to write a cursor.
Either way will work and micro-managing between these two options seems like a waste of time. If you have a manager that is managing this level of detail in your syntax, find a better manager.
March 27, 2008 at 7:10 am
Well - I wouldn't use either. I agree with Grant that a BREAK is not a great way to code it, and...#2 is wrong altogether (if you test for fetch status OTHER than 0, you're looping on error codes...:))
Actually - there's a hybrid solution that looks better to me (in that highly theoretical situation where a cursor declaration is actually appropriate - I HATE those things...)
Declare @fetchstatus int;
set @fetchstatus=0
DECLARE tblcur CURSOR STATIC LOCAL FOR (your select statement);
OPEN tblcur
WHILE (@fetchstatus = 0)
BEGIN
FETCH tblcur INTO (field list);
SET @fetchstatus=@@fetch_status;
IF (@fetchstatus=0)
BEGIN
(do whatever you need to do)
END
END
Close tblCur
DEALLOCATE tblcur
So you don't have an infinite loop you have to BREAK, and you have one single FETCH. The @fetchstatus variable also allows you to not have to worry about where the FETCH is.
And - it will actually loop through your results, too (always a good thing:))
----------------------------------------------------------------------------------
Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?
March 27, 2008 at 8:50 am
Matt,
Yes, the @@fetch_status test was mistyped in the second example - proves that code should never be cut-and-pasted.
I appreciate your thoughtful answer: rather than telling me not to use a cursor, you gave a useful answer to my question.
Thank you,
Chris Judge
March 27, 2008 at 8:58 am
cjudge (3/27/2008)
Matt,Yes, the @@fetch_status test was mistyped in the second example - proves that code should never be cut-and-pasted.
I appreciate your thoughtful answer: rather than telling me not to use a cursor, you gave a useful answer to my question.
Thank you,
Chris Judge
Don't get too effusive...I just figured you'd been beaten up enough about "don't use cursors".:P
I avoid them like the plague as well...:)
Thanks for the feedback!
----------------------------------------------------------------------------------
Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?
March 27, 2008 at 9:11 am
The second is what MS demonstrates and would be what I use if I have to.
My only concern is the first if something goes wrong with the test for condition to break. Because the WHILE is infinite with the 1 = 1 it is more likely the coder will accidentally fail a check and end up in a loop. Bad idea to setup a condition where an infinite loop could get out of hand.
March 28, 2008 at 1:14 pm
In defense of the first method (using WHILE 1=1):
1. Steve McConnell advocates this syntax (in Code Complete) when the language does not natively support a "Loop with test in the middle."
http://www.amazon.com/Complete-Microsoft-Programming-Steve-McConnell/dp/1556154844
2. Erland Sommarskog, a MS MVP whose articles have been features here at SSC, also uses this syntax.
http://www.sommarskog.se/dynamic_sql.html
http://www.sommarskog.se/sqlutil/aba_lockinfo_sql65.sp
3. Umachandar Jayachandran, an engineer on the SQL Server team at MS also uses this syntax.
http://www.umachandar.com/technical/SQL70Scripts/HTMLProject/Main33.htm
April 2, 2008 at 9:12 am
I don't think it actually matters. Similar to arguing the benefits of do.... while vs while...wend statemtns in VB.
Either works, each also requires slightly different syntax and setup (i.e. pre-loading one record from the cursor before hitting the while loop vs doing it at the top of the while 1=1 loop).
Personally, I hate the Break statement. Reminds me of goto.
Viewing 10 posts - 1 through 9 (of 9 total)
You must be logged in to reply to this topic. Login to reply