Worst Practice - Bad Comments

  • vyaskn

    Old Hand

    Points: 344

    quote:


    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!!!


    There's more to it. If you are a vendor and ship your database code to your clients' place, your procedures will fail to run on a case sensitive SQL Server. It is important to be consistent and correct in the usage of case, if you want your code to work at all your client sites.

    HTH,

    Vyas

    http://vyaskn.tripod.com/


    HTH,
    Vyas
    SQL Server MVP
    http://vyaskn.tripod.com/

  • sushila

    SSC-Dedicated

    Points: 35293

    You learn something new every day - since I've never come across a case-sensitive instance of Sql server ever before (limited experience ??) I wasn't even aware that the option to create one existed.

    BTW - why would anyone want to create a case-sensitive instance of sql server - at first I thought it could be for a similar reason as declaring an "option explicit" in vb - but that really doesn't make any sense ???







    **ASCII stupid question, get a stupid ANSI !!!**

  • Steve Rosenbach

    SSCrazy

    Points: 2121

    A very good thread, thanks again Andy!

    I do like to include a header, for many of the same reasons others have mentioned. I've included a sample of a typical one I've been using for the past 9 months that's worked out pretty well when either our new developer or I have to go back and look at a procedure.

    In light of Andy's article, I think I can start eliminating the name of the procedure in the first line of the comments 🙂

    I like to include a brief description of what the procedure is for and any "interesting" things going on inside that might not be obvious from a first or second reading.

    Then I list any parameters and what they mean or any limitations on them.

    I usually list table aliases, and finally, I include a "depends on" section -- but now I'm wondering if I should add a section of what depends on the SP.

    I don't put a list of variables, because I try to create variables that are self-explanatory by their names and context and use functions to aid in clarity, such as

    SET @Qtr1StartDate = FirstDayOfMonthYearAgoThisMonth(@RunDate)

    SET @Qtr2StartDate = DateAdd(qq, 1, FirstDayOfMonthYearAgoThisMonth(@RunDate))

    I think I can begin leaving off the "modifications" comments - you guys are right - that's easier and better done via source control.

    Best regards,

    SteveR

    /* **************************************************************************************

    spRptDoDByCutoffBasis

    Usage: EXEC dbo.spRptDoDByCutoffBasis 42782, '4/11/02', 'S', 'F'

    Description: Outputs the PXZX and Units/1000 for the immediate past year

    and the previous year, figured from the RunDate parameter.

    The FromDate of the immediate past year is 1 year prior to

    the first day of the RunDate month. The ToDate of the

    immediate past year is the last day of the month preceding

    the RunDate. The corresponding parameters for the "LastYear"

    dates are 1 year earlier

    Parameters: @SavedReportID - PK of record in table WH_SavedReports - used

    to provide lists of Bases included, Departments included, etc.

    @RunDate - used to calculate From and To dates for immediate past year

    and the previous year

    @RegularOrSpecialCount - 'R' for PRZX, 'S' for PSZX. Defaults to 'R'

    @UseStartOrFinishDate - whether data is grabbed based on Start ('S') or

    FinishDate ('F')

    Table Aliases: c = WH_1Processes_xxxx

    d = WH_ParentCodes

    p = WH_Procedures

    Depends On: Function dbo.spGetRegularAndSpecialCounts (therefore requires SQL Server 2000 or higher)

    By : S. Rosenbach 09/12/2002

    Modified : S. Rosenbach 09/23/2002 - added ability to run using either Start or Finish Date

    S. Rosenbach 10/03/2002 - modified definition of running as Finish to mean finished by

    the Run Date AND Posted by the current date.

    *********************************************************************************** */

  • Steve Rosenbach

    SSCrazy

    Points: 2121

    whoops, talk about a good example of things not retaining original spacing when you cut and paste... my header example lost all of my laborious indentation.

    -- SteveR

  • Andy Warren

    SSC Guru

    Points: 119694

    So the consensus is for headers then? Minus indents (Steve R)?

    Anyone storing more proc meta data elsewhere? Other than VSS I mean.

    Andy

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

  • wodom

    Hall of Fame

    Points: 3813

    As long as we're talking about comments, here's my #1 rule: The most important word in a comment is "because" (or appropriate synonyms, of course). It's even more necessary to document the why of something than the what.

  • abhi_develops

    Say Hey Kid

    Points: 676

    The system that we have inherited follows most of the worst-practices mentioned at SQLServerCentral.com

    Especially so with comments. We used to have too many comments till we introducted documentation. Otherwise most of the comments were commented out code which used to work earlier. I definitely support the other readers regarding the use of headers. I feel that the headers are really important. We also make it a point to mention the callee program in the header.

    Thanks,

    Abhijit

  • dweil

    SSCrazy

    Points: 2408

    The reason a lot of applications require case sensitivity it because the apps were originally written on Unix Platforms where case sensitivity is taken for granted.

    For example ERP systems like Peoplesoft and Lawson. The downside of this ability to run on multiple platforms is some incredibly horrible sql that works on every rdbms on every platform.

    You learn something new every day - since I've never come across a case-sensitive instance of Sql server ever before (limited experience ??) I wasn't even aware that the option to create one existed.

    BTW - why would anyone want to create a case-sensitive instance of sql server - at first I thought it could be for a similar reason as declaring an "option explicit" in vb - but that really doesn't make any sense ???

  • Andy Warren

    SSC Guru

    Points: 119694

    Porting from another DBMS that is case sensitive?

    Andy

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

  • David.Poole

    SSC Guru

    Points: 75393

    I've set up a case-sensitive SQL Server simply because the data that had to be imported from external sources needed to retain its case sensitivity. ID fields were an unholy mix of case sensitive alphanumerics.

    If the tables had been small I would have installed SQL as case insensitive and cast to varbinary but as they ran into several million records a piece I stuck with case sensitivity.

    Note that you get some pretty hefty indexes with case sensitivity.

  • Rigsby

    SSC Rookie

    Points: 37

    Lots of comments about neatness and readability which are all very good, but slightly missing the point, I feel.

    Two points:

    1. The purpose of a comment (more often than not) is first and foremost to tell you <b>why</b> something is being done and possibly how it is being done if that is not already obvious. If you can't explain your code, then either it's not doing what it should, or perhaps it could be simplified.

    2. Comments are not there to give a running commentary on your code, and it is developers who see them this way who treat them as an optional extra.

  • nmoore

    SSCrazy

    Points: 2304

    Version control?

    I've seen production servers where retrieving an older veriosn of stored proc. would mean getting the backup of the night before and recreating the database.

    Nigel Moore
    ======================

  • Andy Warren

    SSC Guru

    Points: 119694

    Not to argue (much), but I dont think its always feasible to make your code self explaining. To some degree a good solid name for the proc is the best place to start. I like good long descriptive proc names. Within that, it depends on the code. If you've got a multi hundred line proc, its nice to get an idea up front about issues involved in solving the problem.

    I think the comment about comments not there to track revisions is valid. Have a habit of doing it myself, especially since procs not directly under source control where I can see/add check in comments. I dont see any harm in adding them - sometimes knowing why or when can be pretty powerful aids in figuring something out months later.

    Andy

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

  • mcroswait

    SSC Rookie

    Points: 43

    quote:


    BTW - why would anyone want to create a case-sensitive instance of sql server


    We are in the process of implementing a BaaN/DataSweep ERP - our out-of-the-box DataSweep tables/procs are heavily case-sensitive. I have no idea why.

  • Andy Warren

    SSC Guru

    Points: 119694

    Just think of the fun you'll have with that!

    Andy

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

Viewing 15 posts - 16 through 30 (of 38 total)

You must be logged in to reply to this topic. Login to reply