Click here to monitor SSC
SQLServerCentral is supported by Red Gate Software Ltd.
 
Log in  ::  Register  ::  Not logged in
 
 
 
        
Home       Members    Calendar    Who's On


Add to briefcase 1234»»»

Worst Practice - Bad Comments Expand / Collapse
Author
Message
Posted Saturday, January 04, 2003 12:00 AM
SSCertifiable

SSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiable

Group: Moderators
Last Login: Today @ 7:39 AM
Points: 6,705, Visits: 1,677
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
Post #9095
Posted Wednesday, January 22, 2003 9:50 PM
Forum Newbie

Forum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum Newbie

Group: General Forum Members
Last Login: Wednesday, May 23, 2007 12:59 PM
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.




Post #50804
Posted Wednesday, January 22, 2003 11:55 PM
Grasshopper

GrasshopperGrasshopperGrasshopperGrasshopperGrasshopperGrasshopperGrasshopperGrasshopper

Group: General Forum Members
Last Login: Saturday, July 07, 2012 4:08 PM
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.




Post #50805
Posted Thursday, January 23, 2003 2:40 AM
SSCrazy

SSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazy

Group: General Forum Members
Last Login: Today @ 1:09 AM
Points: 2,863, Visits: 1,703
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
Post #50806
Posted Thursday, January 23, 2003 2:56 AM
Grasshopper

GrasshopperGrasshopperGrasshopperGrasshopperGrasshopperGrasshopperGrasshopperGrasshopper

Group: General Forum Members
Last Login: Thursday, April 22, 2004 2:09 AM
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.





Post #50807
Posted Thursday, January 23, 2003 4:15 AM
SSCrazy

SSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazy

Group: General Forum Members
Last Login: Monday, December 02, 2013 4:20 PM
Points: 2,608, Visits: 29
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.



Post #50808
Posted Thursday, January 23, 2003 4:34 AM
SSCertifiable

SSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiable

Group: Moderators
Last Login: Today @ 7:39 AM
Points: 6,705, Visits: 1,677
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
Post #50809
Posted Thursday, January 23, 2003 8:30 AM
SSCrazy

SSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazy

Group: General Forum Members
Last Login: Monday, March 31, 2014 10:05 AM
Points: 2,553, Visits: 559
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 !!!**
Post #50810
Posted Thursday, January 23, 2003 8:41 AM
SSCrazy

SSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazy

Group: General Forum Members
Last Login: Today @ 1:09 AM
Points: 2,863, Visits: 1,703
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
Post #50811
Posted Thursday, January 23, 2003 3:24 PM
SSCrazy

SSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazy

Group: General Forum Members
Last Login: Monday, December 02, 2013 4:20 PM
Points: 2,608, Visits: 29
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.



Post #50812
« Prev Topic | Next Topic »

Add to briefcase 1234»»»

Permissions Expand / Collapse