|
Found this in a sp, not sure why it was done this way, @fileLength and @userId are passed in, and i've changed the names of the tables so i could post this, so i realize the tables may not make sense(they do in the real code).
DECLARE file_Cursor CURSOR FOR SELECT Length
FROM Files f
INNER JOIN User p
ON p.UserId = f.UserId
INNER JOIN Playlist pm
ON pm.ListId = p.DefaultListId
WHERE p.UserId = @userId
AND pm.FileNumber <> -1
OPEN file_Cursor
FETCH NEXT FROM file_Cursor INTO @length
WHILE @@FETCH_STATUS = 0
BEGIN
SET @fileLength = @fileLength + @length
FETCH NEXT FROM file_Cursor INTO @length
END
CLOSE file_Cursor
DEALLOCATE file_Cursor
Please remember to rate helpful or unhelpful answers, it lets us and people reading the forums know if our answers are any good.
|
|
|
|
|
DBA should be shot for having allowed this to persist in his/her server!
|
|
|
|
|
we dont have an offical dba, we were trusted to check our own stuff once we proved we were doing things right (small shop). obviously nobody ever checked the proc this was in regardless of my recomending they do so.
Please remember to rate helpful or unhelpful answers, it lets us and people reading the forums know if our answers are any good.
|
|
|
|
|
Well, that's probably what sum(length) would do anyway...
Sigh. Good find, good fix.
|
|
|
|
|
Because he/she was bored or they get paid by the number lines of code produced.
|
|
|
|
|
SomeGuyThatIsMe wrote: Found this in a sp, not sure why it was done this way
People new to SQL who are use to standard declarative languages don't realize how sets work. They find it difficult to think that way even presuming they know that it exists.
So they create loops.
|
|
|
|
|
I wouldnt have posted, well ok maybe i would have, if this was a new or junior person. If they were still around i would have tried to help them. This was written near the end of their time with the company, maybe that explains it. This person also spent a lot of time trying to prove how much smarter they were than everyone else.
Please remember to rate helpful or unhelpful answers, it lets us and people reading the forums know if our answers are any good.
|
|
|
|
|
SomeGuyThatIsMe wrote: I wouldnt have posted, well ok maybe i would have, if this was a new or junior
person
I said "new to SQL" and not new to programming.
And that is what I meant. Someone new to programming might be more likely to do it correctly because they don't already have expectations of how it 'should' be done. But these days either is likely to due a google search an implement it using the first example they find because neither has the necessary knowledge to filter it out.
|
|
|
|
|
Wow, talk about doing it the hard (and wrong) way. I suspect that this is a symptom of the state of on-the-job training in the programming world. It usually goes like "hey here's a new programmer, do this." But I still don't see how one becomes a programmer without learning the basics of SQL. No one likes code reviews, but it's useful for things like this.
Also, I love how @@FETCH_STATUS is global. I once saw an expensive production system blow-up due to a legacy code time bomb in the form of a SP using a global variable like this.
|
|
|
|