Today I was reviewing some commits by our team and I ran into one commit in general that first made me vomit a bit in my mouth, then get pissed as all hell that we committed code like this.
The code that was committed was a minor change to one of our ‘legacy’ (aka has no tests, written like a blind monkey coded it and god help the person who has to make changes to it) stored procs. Here are some details on the proc
Has about 2050 lines of code
Has about 141 variables
Has 6 Temp tables
Has > 50 update statements
Has ~13 delete statements
Has 8 Cursors
What makes me so mad is that this code when it was originally written was code reviewed by a senior developer on the team (I know this be cause like many other old school shops at that time we put our commit comments in the header of every file). I am going to bet the farm that had this been C# code and not SQL this would not have passed review. BUT for what ever reason SQL seems to be treated differently. When writing SQL many people throw best practices out the window, and I have NO clue why.
The one message I want to get across to anyone out there reading this is…. Code is Code and coding concepts are cross language. If while writing .net/java/ruby/etc code you follow practices like SRP, DRY, etc then why not follow those same principles when writing code in SQL (or any language for that fact)
Till next time
03-01-2010 5:49 AM