SQL Clone
SQLServerCentral is supported by Redgate
 
Log in  ::  Register  ::  Not logged in
 
 
 


Writing Maintainable Code


Writing Maintainable Code

Author
Message
Pam Brisjar
Pam Brisjar
SSCarpal Tunnel
SSCarpal Tunnel (4.9K reputation)SSCarpal Tunnel (4.9K reputation)SSCarpal Tunnel (4.9K reputation)SSCarpal Tunnel (4.9K reputation)SSCarpal Tunnel (4.9K reputation)SSCarpal Tunnel (4.9K reputation)SSCarpal Tunnel (4.9K reputation)SSCarpal Tunnel (4.9K reputation)

Group: General Forum Members
Points: 4866 Visits: 2804
Comments posted to this topic are about the content posted at http://www.sqlservercentral.com/columnists/pabdulla/writingmaintainablecode.asp
David le Quesne
David le Quesne
SSCrazy
SSCrazy (2.1K reputation)SSCrazy (2.1K reputation)SSCrazy (2.1K reputation)SSCrazy (2.1K reputation)SSCrazy (2.1K reputation)SSCrazy (2.1K reputation)SSCrazy (2.1K reputation)SSCrazy (2.1K reputation)

Group: General Forum Members
Points: 2149 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...
Jeff Moden
Jeff Moden
SSC Guru
SSC Guru (340K reputation)SSC Guru (340K reputation)SSC Guru (340K reputation)SSC Guru (340K reputation)SSC Guru (340K reputation)SSC Guru (340K reputation)SSC Guru (340K reputation)SSC Guru (340K reputation)

Group: General Forum Members
Points: 340914 Visits: 42650

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.
If you think its expensive to hire a professional to do the job, wait until you hire an amateur. -- Red Adair

Helpful Links:
How to post code problems
How to post performance problems
Forum FAQs
MattieNH
MattieNH
SSCarpal Tunnel
SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)

Group: General Forum Members
Points: 4379 Visits: 901

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





JT Lovell
JT Lovell
SSC-Addicted
SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)

Group: General Forum Members
Points: 422 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.


Paul Chapman-279652
Paul Chapman-279652
SSC-Addicted
SSC-Addicted (420 reputation)SSC-Addicted (420 reputation)SSC-Addicted (420 reputation)SSC-Addicted (420 reputation)SSC-Addicted (420 reputation)SSC-Addicted (420 reputation)SSC-Addicted (420 reputation)SSC-Addicted (420 reputation)

Group: General Forum Members
Points: 420 Visits: 125

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.


MattieNH
MattieNH
SSCarpal Tunnel
SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)SSCarpal Tunnel (4.4K reputation)

Group: General Forum Members
Points: 4379 Visits: 901

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





JT Lovell
JT Lovell
SSC-Addicted
SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)SSC-Addicted (422 reputation)

Group: General Forum Members
Points: 422 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;


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