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

Writing Maintainable Code Expand / Collapse
Author
Message
Posted Thursday, July 13, 2006 9:29 PM
SSC Eights!

SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!

Group: General Forum Members
Last Login: Thursday, May 1, 2014 7:26 AM
Points: 908, Visits: 2,804
Comments posted to this topic are about the content posted at http://www.sqlservercentral.com/columnists/pabdulla/writingmaintainablecode.asp
Post #294301
Posted Monday, August 7, 2006 1:51 AM


SSC Eights!

SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!

Group: General Forum Members
Last Login: Tuesday, August 10, 2010 4:54 AM
Points: 815, Visits: 32

Good common sense Pam! Thanks!

I'd like to add another one please.

Make the code modular (if your system will support it).

Extract frequently used functionality to a separate code module, and call that module where-ever you need to perform the function. This will be far easier to test and maintain than hunting down umpteen different attempts at the same function written by different programmers at different times (and even in different languages!). If you can, write the module so that it is dependent on input of only the minimum of key fields and the data to be processed. For everything else it should be self-sufficient. This may mean it has to look up a few things along the way, but you will be able to make far wider use of it than if it is dependant on a whole gaggle of inputs.

Then document the function so all your programming colleagues know about it. Don't keep it to yourself!

David



If it ain't broke, don't fix it...
Post #299887
Posted Monday, August 7, 2006 6:13 AM


SSC-Dedicated

SSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-Dedicated

Group: General Forum Members
Last Login: Yesterday @ 8:58 PM
Points: 36,794, Visits: 31,253

quote

A basic rule of thumb: if you have to comment the code to explain what you did and why, you should seriously consider rewriting the code.

I was with you all the way until you said that... why is it that people are so against simple embedded documentation?

A simple comment for each SELECT, INSERT, UPDATE, DELETE work miracles when you have to modify someone else's code.  Something like...

--===== Mark qualifying customers for rewards for the month

... takes no time at all to write and the reader immediately knows whether or not this section of code has anything at all to do with the required mod... in many cases, the might mean the reader can skip the next hundred lines of code without worry...

...and, quick, what does a WHERE SvcTypeID IN (1,2,3,5,1801) mean?  If you are not familiar with the code, it may take you hours to find the right person or the right table (if there is one).  If you are familiar with the code, it takes scant seconds to do this...

WHERE SvcTypeID IN (1, --Standalone Long Distance
                    2, --800 Service
                    3, --800 Pin

                    5, --Calling Card
                 1801  --Bundled Services
                    )

By the way, I'm sometimes surprised when the ladies use "Rule of Thumb"... they don't realize that it's an old term for how thick a stick you could use to legally to beat your wife with.  Maybe, if it were documented, they wouldn't use it as much



--Jeff Moden
"RBAR is pronounced "ree-bar" and is a "Modenism" for "Row-By-Agonizing-Row".

First step towards the paradigm shift of writing Set Based code:
Stop thinking about what you want to do to a row... think, instead, of what you want to do to a column."

(play on words) "Just because you CAN do something in T-SQL, doesn't mean you SHOULDN'T." --22 Aug 2013

Helpful Links:
How to post code problems
How to post performance problems
Post #299917
Posted Monday, August 7, 2006 7:22 AM
SSCrazy

SSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazy

Group: General Forum Members
Last Login: Tuesday, July 29, 2014 10:26 AM
Points: 2,592, Visits: 782

Jeff,

People are against simple embedded documentation for at least two reasons.  First, comments tend to state the obvious.  So if you establish a standard that requires comments, you get things like this:

--Update employee's name
UPDATE Employee
Set FirstName = @FirstName, LastName = @LastName

The other reason is that comments don't tend to get modified when the code does.  So you run the risk of being misled by documentation when looking at the code is what you really need to do.

And keep in mind that these are standards for all development, not just SQL.  Languages like COBOL and VB have paragraph or module names that should describe what is happening in that paragraph or module, like RenameExistingFile.  Since that piece of code should do nothing more than rename an existing file, what kind of comments do you need?  In your example above, I'd name the stored procedure stp_MarkForMonthlyCustomerRewards.

I think it's important to document code that is either nonintuitive or counter-intuitive.  It should help, and at least the next person will know you were thinking something when you did it, rather than randomly generating irrelevant code.

And when you say "in many cases, the might mean the reader can skip the next hundred lines of code without worry", if I saw hundreds of lines of code in any module, paragraph, or stored procedure, I'd already be worried.

Mattie




Post #299927
Posted Monday, August 7, 2006 8:22 AM


Valued Member

Valued MemberValued MemberValued MemberValued MemberValued MemberValued MemberValued MemberValued Member

Group: General Forum Members
Last Login: Monday, November 12, 2007 6:32 AM
Points: 62, Visits: 3

Good article Pam!

I'm with Jeff on the issue of comments though.  If you *have* to put comments to tell *what* you did, it's either very complex or poorly designed.  But that doesn't mean you should leave them out.  "Obvious" comments are helpful to junior developers to whom the code will be less obvious.  They often do not understand why one approach was taken instead of another and they're often tasked with maintaining older code long before they get to write new code.  Comments are a great teaching tool.

Comments explaining *why* you did something are needed in every block except the most trivial.  You can't determine why something was done by looking at code.  You can guess, and you might even guess correctly, but you can never be certain because the code only shows the implementation or the *how* something was done.  Comments tell the full story and tie the code to the business process.

If you extract just the comments from my code you will be able to see what and why my code does what it does.  In fact, I usually write the comments as pseudocode before I ever code the statement itself.

And shame on the developer who doesn't update their comments when they update their code.  That's sloppy development and is not excusable.

 

Post #299994
Posted Monday, August 7, 2006 8:26 AM


SSCommitted

SSCommittedSSCommittedSSCommittedSSCommittedSSCommittedSSCommittedSSCommittedSSCommitted

Group: General Forum Members
Last Login: Yesterday @ 7:55 AM
Points: 1,945, Visits: 2,863
That piece of advice is measurably wrong.

Back in my software engineering research days, we found that flowcharts and all the other thigns that were coming out were never as helpful for maintaining code as in-line comments. A program -- even simple ones -- with comments was 12-20% easier to maintain, as measured by the time rerquired to find a planted bug ("be-bugging" was the term used).

If you CANNOT comment the code in a simple sentence that explains what you did and why, you should seriously consider rewriting the code. The code should become obvious to the first-time reader from the comments -- your's was a perfect example.

I will also now shamelssly plug my SQL PROGRAMMING STYLE book that deals with these issues


Books in Celko Series for Morgan-Kaufmann Publishing
Analytics and OLAP in SQL
Data and Databases: Concepts in Practice
Data, Measurements and Standards in SQL
SQL for Smarties
SQL Programming Style
SQL Puzzles and Answers
Thinking in Sets
Trees and Hierarchies in SQL
Post #299997
Posted Monday, August 7, 2006 8:30 AM
Old Hand

Old HandOld HandOld HandOld HandOld HandOld HandOld HandOld Hand

Group: General Forum Members
Last Login: Yesterday @ 8:23 AM
Points: 364, Visits: 119

if i ever get to debug / performance tune something that is written by someone else that is as simple as

update table1

set field1 = value1

, field2 = value2

then i will be truly amazed.

however, i get complicated selects with ridiculous joins that i have to work out why on earth they've done it (and every time they'll have left the company before i get called in).

so i have to spend ages trying to work out what they are trying to achieve just from what their code does - which may not even be correct.

if they'd only given me a few clues i could have not wasted my time and the company's money guessing!

one of the ones recently i had to do included "where isnull(column1, '') = value1" and value1 could never be an empty length string.

Post #299999
Posted Monday, August 7, 2006 8:37 AM
SSCrazy

SSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazySSCrazy

Group: General Forum Members
Last Login: Tuesday, July 29, 2014 10:26 AM
Points: 2,592, Visits: 782

Seeing as the majority seems to prefer commented code, I'd be interested to see how you would document code (presumably SQL) that you've written or maintained.  Maybe once I see a good example of it, I'll be convinced!

Thanks,

Mattie




Post #300004
Posted Monday, August 7, 2006 9:06 AM


SSCommitted

SSCommittedSSCommittedSSCommittedSSCommittedSSCommittedSSCommittedSSCommittedSSCommitted

Group: General Forum Members
Last Login: Yesterday @ 7:55 AM
Points: 1,945, Visits: 2,863
Jeff gave a good example -- explaining codes in a CHECK() constraint or CASE expression.

Programmers who come from a procedural language forget that SQL has three sub-languages and they all need comments. In the DDL, I want to see the validation verification rules of my encodings or I want to have a data dictionary. In the DML, tell me what the module does, who the programmer was and the last update of the code. DCL is a lot easier in one way, but you should have a policy statement ("Do not let salesmen change their own commission rates and base pay", etc.)



Books in Celko Series for Morgan-Kaufmann Publishing
Analytics and OLAP in SQL
Data and Databases: Concepts in Practice
Data, Measurements and Standards in SQL
SQL for Smarties
SQL Programming Style
SQL Puzzles and Answers
Thinking in Sets
Trees and Hierarchies in SQL
Post #300016
Posted Monday, August 7, 2006 9:11 AM


Valued Member

Valued MemberValued MemberValued MemberValued MemberValued MemberValued MemberValued MemberValued Member

Group: General Forum Members
Last Login: Monday, November 12, 2007 6:32 AM
Points: 62, Visits: 3

Hi Mattie,

Here's a quick example I grabbed that is lightly sprinkled with comments but they're enough to guide the reader.  It's not a particularly complex function, but I hope you can see how it gives clues about what's going on without requiring the reader to fully understand the code.  Please no one kill me, I realize this is an Oracle function.

CREATE OR REPLACE FUNCTION example.idt_f_get_constant_dt (
/*
|   Unit Name: idt_f_get_constant_dt
|   File Name: idt_functions.sql
|     Purpose: Return a global constant, if a non-date value is encountered, return null
|  Parameters: p_constant_nm   IN  VARCHAR2   the constant name requested
|              p_site_cd       IN  VARCHAR2   the site code requested (defaults to GLOBAL)
|              return          OUT VARCHAR2   the constant value as a date.  If a site
|                                             constant is requested but not found,
|                                             the global constant of the same name is
|                                             returned.  Null if no value is found or
|                                             the value is not a date.
|
|  Who:  Date:       Ver:    Description (References):
|  JTL   05/03/2006  1.0.0   Initial Build
|  JTL   06/25/2006  1.14.0  Production Release of [system]
|
*/
  p_constant_nm IN VARCHAR2,
  p_site_cd     IN VARCHAR2 := 'GLOBAL'
)
RETURN DATE
IS
 
  v_data_type_cd VARCHAR2(10);
  v_value_txt VARCHAR2(100);
  v_value_dt DATE;
  v_value_nbr NUMBER;
 
  v_count_nbr NUMBER;
  v_return_dt VARCHAR2(100);
 
BEGIN

  /* determine the whether the constant exists */
  SELECT COUNT(*)
    INTO v_count_nbr
    FROM example.idt_constant c
   WHERE c.constant_nm = p_constant_nm
     AND c.site_cd = p_site_cd;
  
  IF v_count_nbr = 1 THEN
    /* found constant, determine data type */
    SELECT NVL(c.data_type_cd, 'STRING'),
           c.value_txt,
           c.value_dt,
           c.value_nbr
      INTO v_data_type_cd,
           v_value_txt,
           v_value_dt,
           v_value_nbr
      FROM example.idt_constant c
     WHERE c.constant_nm = p_constant_nm
       AND c.site_cd = p_site_cd;

    /* set return value */ 
    IF v_data_type_cd = 'STRING' THEN
      v_return_dt := NULL;

    ELSIF v_data_type_cd = 'DATE' THEN
      v_return_dt := v_value_dt;

    ELSIF v_data_type_cd = 'NUMBER' THEN
      v_return_dt := NULL;
 
    ELSE
      v_return_dt := NULL;
 
    END IF;     
 
  /* did not find the site-specific constant, so check for a global version */
  ELSIF p_site_cd <> 'GLOBAL' THEN
    v_return_dt := 
      example.idt_f_get_constant_dt(p_constant_nm => p_constant_nm, p_site_cd => 'GLOBAL');

  /* did not find the global constant */     
  ELSE
    v_return_dt := NULL;
 
  END IF;
   
  /* return the value */
  RETURN v_return_dt;

EXCEPTION
 WHEN NO_DATA_FOUND THEN
   RETURN NULL;
 
END idt_f_get_constant_dt;

Post #300020
« Prev Topic | Next Topic »

Add to briefcase 1234»»»

Permissions Expand / Collapse