Click here to monitor SSC
SQLServerCentral is supported by Redgate
 
Log in  ::  Register  ::  Not logged in
 
 
 


Worst Practice - Bad Comments


Worst Practice - Bad Comments

Author
Message
Andy Warren
Andy Warren
SSCertifiable
SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)

Group: Moderators
Points: 7265 Visits: 2680
Comments posted to this topic are about the content posted at http://www.sqlservercentral.com/columnists/awarren/worstpracticebadcomments.asp>http://www.sqlservercentral.com/columnists/awarren/worstpracticebadcomments.asp

Andy
SQLAndy - My Blog!
Connect with me on LinkedIn
Follow me on Twitter
Eric P. Melillo
Eric P. Melillo
Forum Newbie
Forum Newbie (5 reputation)Forum Newbie (5 reputation)Forum Newbie (5 reputation)Forum Newbie (5 reputation)Forum Newbie (5 reputation)Forum Newbie (5 reputation)Forum Newbie (5 reputation)Forum Newbie (5 reputation)

Group: General Forum Members
Points: 5 Visits: 1
Andy - I agree with your view of bad comments. I do however, kick code back to our devs if they do not have PVCS header tags in the body of the stored procedure. I utilize an inhouse sproc that will compare all sprocs, functions and perms between the current instance and another instance to see what changes there are - really helps when the system acts differently between platforms (dev, build, qa, stage and prod)...
My 2 cents worth on comment blocks with version, author, etc in them.



AjarnMark
AjarnMark
Grasshopper
Grasshopper (21 reputation)Grasshopper (21 reputation)Grasshopper (21 reputation)Grasshopper (21 reputation)Grasshopper (21 reputation)Grasshopper (21 reputation)Grasshopper (21 reputation)Grasshopper (21 reputation)

Group: General Forum Members
Points: 21 Visits: 55
I agree in part. I like top/bottom borders for "major sections" (define for yourself what constitutes major). I don't have many sprocs with major sections, but I do have a number of maintenance or build scripts with major sections, and I like having the borders to highlight that fact. Perhaps this is anal of me, but I have a template with the borders measured just long enough to go all the way across the page if it is printed, and not wrap to a second line. But I agree with getting rid of left/right borders. Too much of a pain.

I like a comment block at the top that includes a brief narrative and a revision history, but if the history is really getting lenghy in comparison to the rest of the code, perhaps it should be truncated or summarized. I have seen, and do NOT like, date-stamped comments about each minor change within the code. Similar to the "old code" comment, it tends to overrun the code and make it hard to follow what is really going on. When I want to know exactly what changed, I go back to my version control system and run a Diff comparison.



David.Poole
David.Poole
Hall of Fame
Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)

Group: General Forum Members
Points: 3708 Visits: 3121
I insist on header blocks because a well written header tells the uninitiated at a glance what the proc/function is supposed to do.
I do keep a history in the proc, but truncate it if it gets too excessive.

I like to detail
  • Variables used
  • Arguments Used
  • Tables/Views used
  • Procs/Function used


There is nothing worst than scrabbling around a large stored proc trying to figure out what variable @x is supposed to represent.

If you are looking at an app regularly or you wrote the app then header comments are of no particular advantage. If, however, you are a new comer to the app, or you haven't worked on an app for some time the use of headers becomes a godsend.

LinkedIn Profile

Newbie on www.simple-talk.com
mselway
mselway
Grasshopper
Grasshopper (10 reputation)Grasshopper (10 reputation)Grasshopper (10 reputation)Grasshopper (10 reputation)Grasshopper (10 reputation)Grasshopper (10 reputation)Grasshopper (10 reputation)Grasshopper (10 reputation)

Group: General Forum Members
Points: 10 Visits: 1
I agree with the bulk of the article, however, headers are in my opinion essential.
To have a description of the purpose of the procedure, alongside the code, is the one piece of documentation that is not going to be lost.



NPeeters
NPeeters
Hall of Fame
Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)

Group: General Forum Members
Points: 3081 Visits: 36
Also like the comment header. Agree with David.Poole on this. And we agreed to cut out all changes in between major versions / releases.

This keeps the changelog nice and short.



Andy Warren
Andy Warren
SSCertifiable
SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)SSCertifiable (7.3K reputation)

Group: Moderators
Points: 7265 Visits: 2680
Good comments!

Sorry, couldnt help myself!

One thing I've been considering trying is adding a 'Used By' comment at the top that would indicate any app that calls the proc. If kept up, you'd know instantly the real dependencies, not just the ones SQL knows about (and it doesnt always have them all even for procs that call other procs). Anyone tried anything like this?


Andy
http://www.sqlservercentral.com/columnists/awarren/

Andy
SQLAndy - My Blog!
Connect with me on LinkedIn
Follow me on Twitter
sushila
sushila
SSCrazy
SSCrazy (2.6K reputation)SSCrazy (2.6K reputation)SSCrazy (2.6K reputation)SSCrazy (2.6K reputation)SSCrazy (2.6K reputation)SSCrazy (2.6K reputation)SSCrazy (2.6K reputation)SSCrazy (2.6K reputation)

Group: General Forum Members
Points: 2565 Visits: 639
I agree with all the reader comments on headers and their importance - but essentially I wanted to mention another bad commenting practice that is related to "neatness" and readability - I have come across both code & comments where the developer has used an unhappy & random mix of capital letters and lower case letters (eg: FIRST_name, last_NAME, middle_initial etc...) and is consistently inconsistent throughout the code. The code itself is excellent, but the readability is so painful (& for me traumatic) that I can't seem to go beyond just the sight of the whole thing.
So for all those out there who randomly use lower case and upper case and argue that the code works just fine regardless of what you use - refer to Andy Warren's line where he says that neatness contributes to readabilty - & readability finally contributes to maintenance - Just like you wouldn't trash your house with litter, look upon your code the same way - take pride both in it's functionality AND appearance!!!







**ASCII stupid question, get a stupid ANSI !!!**
David.Poole
David.Poole
Hall of Fame
Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)Hall of Fame (3.7K reputation)

Group: General Forum Members
Points: 3708 Visits: 3121
Good point on readability.

One problem with comments is that they can be displayed in a variety of fonts so if you have anything lined up results can be unpredictable.

Ditto indentation of code.

I've tried a recommended display font line in the header.

I tried a "Used By" comment box. The problem is that there has to be a gate keeper if this is to be maintained.

On a large project I tend to develop a series of interface stored procs and publish their intended use to my developers, they can then use these as they require, but as they can use it for any number of routines this makes a "Used By" comment difficult to maintain.

Really, keeping track of who uses what has to be an end to end process practised by the organisation, not just one or two individuals in a team.

Basically in small organisations you can have an "ad-hocracy". Operations under ad-hocracies are fast and flexible. The problem comes when you grow beyond a certain point ah-hocracies implode.

LinkedIn Profile

Newbie on www.simple-talk.com
NPeeters
NPeeters
Hall of Fame
Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)Hall of Fame (3.1K reputation)

Group: General Forum Members
Points: 3081 Visits: 36
Again, have to agree with David. 'Used By' comments are usefull, but it's a pain to keep them up to date. And if you can't trust them, what's their use.

In that respect, I tend to look at procedures (and other SQL scripts) as 'ordinary OO' code, with the appropriate lessons to follow on high cohesion / low coupling.

If we are developping for a large project, we try to add an indication of 'module' to the name of each object. One person is responsible for each module, so he makes sure nothing breaks inside a module after changes.
For 'inter module' communication, we design an interface, consisting of views and insert / update / delete procedures, that has to be adhered to.

If someone has to add a field to a table, he has to make sure all of the interface objects will keep their functionality. Hurray to default values and ban all 'SELECT * FROM ORDER BY 1' kind of stuff

This kind of set-up has worked for our biggest projects, involving up to four different firms with in total some twenty concurrent developpers in SQL, C++, VB and Crystal / Cognos.



Go


Permissions

You can't post new topics.
You can't post topic replies.
You can't post new polls.
You can't post replies to polls.
You can't edit your own topics.
You can't delete your own topics.
You can't edit other topics.
You can't delete other topics.
You can't edit your own posts.
You can't edit other posts.
You can't delete your own posts.
You can't delete other posts.
You can't post events.
You can't edit your own events.
You can't edit other events.
You can't delete your own events.
You can't delete other events.
You can't send private messages.
You can't send emails.
You can read topics.
You can't vote in polls.
You can't upload attachments.
You can download attachments.
You can't post HTML code.
You can't edit HTML code.
You can't post IFCode.
You can't post JavaScript.
You can post emoticons.
You can't post or upload images.

Select a forum

































































































































































SQLServerCentral


Search