SQLServerCentral / Discuss Content Posted by Pam Abdulla / Article Discussions / Article Discussions by Author / Writing Maintainable Code / Latest Posts

RE: Writing Maintainable Code

Jeff Moden — Wed, 08 Aug 2007 18:12:00 GMT

That's actually a pretty good plan and I agree with that... nice to see someone to spend a little time up front to save wads later on.

RE: Writing Maintainable Code

Mike Dougherty-384281 — Wed, 08 Aug 2007 11:59:00 GMT

nearly never. Absolutely never in a public example of maintainable code. I forced myself to develop a habit of taking the minor pain up front to set up and use the DB for data rather than the major pain returning to the project months later and having to figure things out.

RE: Writing Maintainable Code

Jeff Moden — Tue, 07 Aug 2007 17:50:00 GMT

Heh... I asked the same thing... and the answer is... crappola 3rd party software.

So, Mike, you never hardcode anything?

RE: Writing Maintainable Code

Mikey-85319 — Tue, 07 Aug 2007 07:50:00 GMT

Just weighing in... good points in the article... I think some comments have their place, for example when you do something strange in the code for a good reason. An example I used recently is I had to create an in-line query that was nested twice. It was very unpleasant but it did the job. I commented it carefully so that developers who had to maintain the code could understand the thought process and why it was done like that.

My only pet-peeve regarding comments is when developers comment out reams of code and LEAVE IT IN THE PROGRAM. Trying to read and decipher that is a difficult task. It is even more pointless if you are using a version maintenance tool, such as SourceSafe. Just remove it, add a comment if you must indicating that code was removed and refer to SourceSafe. When I have to maintain a new program, I do 2 things, one is add whitespace, as article author indicated, and two is delete commented out code.

MikeZane

RE: Writing Maintainable Code

Mike Dougherty-384281 — Tue, 07 Aug 2007 07:37:00 GMT

WHERE SvcTypeID IN (1, --Standalone Long Distance 2, --800 Service 3, --800 Pin 5, --Calling Card 1801 --Bundled Services )whether this is commented "properly" or not, I ask why these values are not in a table. I know the knee-jerk response is that it's easier/faster to use this construct, but how often has easier/faster for the developer been the long term growth solution? If these values do not exist in a control table, why not? If they ARE, then why is the developer hard-coding this list of services into the procedure? In an article on maintainable SQL, I would have expected hard-coding to be absolutely forbidden.

RE: Writing Maintainable Code

JT Lovell — Tue, 07 Aug 2007 06:17:00 GMT

>>If you check VB.NET or C# developers, they all write in good maintainable code

Being a .NET developer I can safely say that they do not all write in good maintainable code.

RE: Writing Maintainable Code

Jeff Moden — Tue, 07 Aug 2007 05:06:00 GMT

Curious... if you like the way it's done in .Net or C#, why didn't you do it that way?

RE: Writing Maintainable Code

ApieGoku — Tue, 07 Aug 2007 01:39:00 GMT

I also agree, that you must write SQL code maintainable. But NOT on the way they describe in this article. If you check VB.NET or C# developers, they all write in good maintainable code. If they use SQL, then it is an disaster. I have some good samples:

When you use columns in a select statement, place the comma at firtst. Now you can easely comment a column:

SELECT ColumnA

, ColumnB

-- , ColumnC

FROM TABLE1 WITH (NOLOCK)

WHERE STATEMENT1 = @p_PARAMETER

AND STATEMENT2 = @l_PARAMETER

I also places all columnnames on the same place and all signs and parameters. Now you can easily read the code.

RE: Writing Maintainable Code

Jeff Moden — Tue, 08 Aug 2006 05:04:00 GMT

The point is, Mark, instead of having to read through the code to find what you want, you only have to lightly scim through the comments to find out what a section of code does and WHY it was written. If you are totally unfamiliar with the code (someone else wrote it) and you've been assigned to do a light mod to the code, it can save you a bit of time.

You can read code... without looking back at JT's code, WHAT does this do? You have 3 seconds, my friend because your boss is standing over your shoulder, knows you can read code because you've been bragging all day that "if you cant do that then you shouldnt be any where near the code", and expects you to find and fix a problem while he or she is waiting... yes, yes... anyone worth their salt can read this and figure out HOW it's doing something but you have to study it for more than 3 seconds to figure out WHY it's doing it...

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;

Now, go back and look at JT's code and you'll find the comment that saves so much time...

/* found constant, determine data type */

The point is, Yes, if you can't read code, I agree, you probably shouldn't be anywhere near the code. If you can't eventually figure out "why" (what the purpose of) some undocumented code was written, I agree... you shouldn't be near it. But, simple little sprinklings of good comments even on "obvious" code is a real time saver when troubleshooting. For non-obvious, complicated, or "tricky" code, you will need more substantial comments.

Then, there are QA people... they are NOT necessarily people who have a deep understanding of how to read code but they do need to be able to quickly map out what a proc is supposed to do so they can setup a test case to figure out if the code actually did what it's supposed to especially if it writes to a table. Simple but well written comments can and do save QA people huge amounts of time.

RE: Writing Maintainable Code

Kiwi_Mark_LFC — Mon, 07 Aug 2006 23:04:00 GMT

meaningless comments!!!none of those comments are required...they can be derived from reading the code...if you cant do that then you shouldnt be any where near the code!the only comments that should be in any program are those that document WHY something was done

RE: Writing Maintainable Code

Jeff Moden — Mon, 07 Aug 2006 20:16:00 GMT

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

Shameless or not, I'm going to have to get a copy of that one... thanks for the tip, Joe...

RE: Writing Maintainable Code

Jeff Moden — Mon, 07 Aug 2006 19:58:00 GMT

1801 --Bundled Services, not including VOIP

your documentation is out of date, and the code hasn't even been touched

Mattie,

That might be true except for one thing (I believe this was one of your previous good points, as well)... the code is in a proc called ApplyAdjToPOTS. On the outside chance that someone doesn't know what "POTS" is (an industry standard acronym), we put it in the header of the code... "POTS: Plain Old Telephone System". We have totally separate systems to drive Wireless, VOIP, and a couple of other goodies.

RE: Writing Maintainable Code

Charles Kincaid — Mon, 07 Aug 2006 16:15:00 GMT

Oh, it may not be that funny.

_sql = "UPDATE Inventory SET QOH = QOH + " & CSTR(ReciptQuantity)

Found this in production code. Customer wondered why his inventory was "over stated".

RE: Writing Maintainable Code

Calvin Lawson — Mon, 07 Aug 2006 15:31:00 GMT

You'd actually want your comment to say that you are updating ALL the employee first and last names to the variable value. That would be an interesting comment! Just teasing...

RE: Writing Maintainable Code

Charles Kincaid — Mon, 07 Aug 2006 14:58:00 GMT

Mattie,

We are of the same mind. I have seen white space reducers for PERL and C++. This all comes from the days when we were storing source code on flippies (You remember: Floppies that you notched so that you could use bothe sides).

Then there were COBOL programmers paid by the line. This yielded the one word per line junk.

VB.Net makes my code pretty. The auto indent helps me spot logic errors before I even try to single step. Go to the line above a function definition and type three tick marks ('''). Instant XML documentation. Just fill in the blanks. I can look at the object browser and not have to open the code to find the right function. (Hey. Subs are functions that return Void)

Comment vs Not Comment. I vote comment. Your comments should be meaningful. Function UpdateOrderHistory(ByVal OrderNumber AS Integer) as Boolean.

' This function updates the order history.

My boss said, "Well, no sh**! I thought it might make coffee or something."

RE: Writing Maintainable Code

Tom Erwine — Mon, 07 Aug 2006 14:16:00 GMT

Mattie,

My comment was in regard to the :

ReadyStatus = ( ReadyStatus = False )

If it took 10 lines of code to do the equivalent, and the above is correct, you shouldn't be adding an extra 9 lines of code because it may not be apparent to a junior programmer. This would handicap you greatly if you cannot use advanced features of the language. As a twenty year vet, I mostly see the opposite, where a junior coder did 10 lines of code where there is already a built in function that did the same thing in 1 line.

RE: Writing Maintainable Code

MattieNH — Mon, 07 Aug 2006 13:38:00 GMT

Tom,

I'm not sure I agree with the concept that the amount of typing should control how code is written. After twenty years of programming, I've realized that you generally pay dearly when doing maintenance for shortcuts taken in development. As one of my professors once asked, 'if you don't have time to do it right, how can you have time to do it over?'.

Mattie

RE: Writing Maintainable Code

Paul Paiva — Mon, 07 Aug 2006 12:56:00 GMT

Another good example of proper usage of comments within code is to look at some of the system stored procedures. In the MS world, try looking at sp_get_composite_job_info located in msdb. I was just in there today trying to gather some meta data of the jobs on the server. I have learned considerably from reviewing these procs, and the comments are extremely helpful.

- Paul

RE: Writing Maintainable Code

Tom Erwine — Mon, 07 Aug 2006 12:44:00 GMT

If it doe not affect speed or resources, I would vote for rewrite since the typing is approximately the same. If any of these 3 are affected (speed, resource, or much more typing) then keep the same. As far as documenting, this is where the question of "what is obvious" comes into play. If too many comments are made for the sake of junior programmers, then the "important" comments get lost.

RE: Writing Maintainable Code

Stephanie J Brown — Mon, 07 Aug 2006 12:42:00 GMT

Put me on the "comment your code" side of the argument (the "why" can be critical six months down the line); and adding white space for readability, too!

Since I work in multiple languages (starting with COBOL back in '80), the "don't use abbreviations" in names is one area I have to disagree with.

If we didn't abbreviate, the COBOL names would get totally out of hand, and take up an entire line by themselves. In my current business, we have standard abbreviations that are used, for instance "Elig" means "Eligibility". Fortunately it's a "spoken abbreviation" as well - a business standard! I'll maintain, for the sake of discussion, that as long as an abbreviation is standardized, it's okay to use them in names of variables, stored procedures, functions, or whatever.

In terms of standards, I agree with Pam that they give you a structure, but getting too detailed with them will cause problems. I worked on a standards committee once where we had a "cross every <T> dot every <I>" member, and it wasted a whole lot of time. A basic set of rules that everyone can understand is essential; anything beyond that is superfluous.

RE: Writing Maintainable Code

MattieNH — Mon, 07 Aug 2006 12:02:00 GMT

ReadyStatus = ( ReadyStatus = False )

So should this be documented, or rewritten?

Mattie

RE: Writing Maintainable Code

Charles Kincaid — Mon, 07 Aug 2006 11:47:00 GMT

Obvious, from reading code?

Try

ReadyStatus = ( ReadyStatus = False )

To an experinced VB programmer it might be appearent but two member of our team could not understand what it did until it was re-written as:

ReadyStatus = NOT(ReadyStatus)

RE: Writing Maintainable Code

bill page-320204 — Mon, 07 Aug 2006 11:41:00 GMT

Agree with the comments about 'why'. Why isn't used to redundantly explain 'what' but why you made certain choices. It does not explain what the code does, but explains how you decided to default something or why you are choosing some construct that might not be obvious. Sometimes it might be "the PM said blah blah is ok, but I don't trust that and am handling the possibility by doing xyz".

RE: Writing Maintainable Code

Charles Kincaid — Mon, 07 Aug 2006 11:35:00 GMT

Agreed. But then, "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." does not take into account the many cases where we are deling with importing (or mining) data from somebodyelses tables.

We try to use a good naming convention. We can use my table documentor to extract the table names, column names, and data types. The tech writers cut and paste this into the system manual and only have to fill in comments in rare occasions.

Then I get the joy of having to go get data from SAP, Great Plains, or J. D. Edwards. Not so bad. Next I get an assignment to deal with some home grown system. Now comments every where!

RE: Writing Maintainable Code

JT Lovell — Mon, 07 Aug 2006 10:30:00 GMT

Mattie,

Both the function name and the column names use standard abbreviations that are in the standards guide. We've found it cuts down on the possible ways of naming a column. For instance first name is always "first_nm" and never "first_name", "FirstName", or "FName". The function names are a bit tougher to decode, but they're also standardized to be:

idt = application prefix (used for functions, procs, packages, tables, views, etc)f = function, as opposed to "sp" for procedure or "pkg" for package (oracle group of related functions)get_constant_dt = verb-noun set to describe what the object does.

The v_ indicates a local variable as opposed to a g_ for global, c_ for constant, or p_ for parameter. In Oracle it prevents accidently re-using a variable or column name and causing very difficult to locate bugs.

RE: Writing Maintainable Code

JT Lovell — Mon, 07 Aug 2006 10:22:00 GMT

Tom,

Good catch! This function is actually one of 3 versions (_dt, _nbr, and _txt) with a parallel designs. I.e. they all have the same if/then statement for consistency sake. But yes, the code could be condensed in this version.

RE: Writing Maintainable Code

Tom Erwine — Mon, 07 Aug 2006 10:08:00 GMT

One other area not touched upon that makes code easier to read is: "When you update code, make sure you do not leave code that is no longer relevent." For example JT used:

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;

instead of

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

ELSE v_return_dt := NULL; END IF;

If I had to guess, the original function (or its original intent) returned information about NUMBER and STRING data types. But I cannot just "guess" when working on code so I need to double check the code to see if I am missing something, which wastes valuable time.

RE: Writing Maintainable Code

MattieNH — Mon, 07 Aug 2006 09:39:00 GMT

To Joe's point: I think Jeff's documentation is an excellent example of my point. When the user decides that

SvcTypeID IN (1,2,3,5,1801)

doesn't mean

WHERE SvcTypeID IN (1, --Standalone Long Distance 2, --800 Service 3, --800 Pin 5, --Calling Card 1801 --Bundled Services )

but instead means

WHERE SvcTypeID IN (1, --Standalone Long Distance 2, --800 Service 3, --800 Pin 5, --Calling Card 1801 --Bundled Services, not including VOIP )

your documentation is out of date, and the code hasn't even been touched. I agree completely that stored procedures need a functional description and a modification history.

And JT, thanks for posting the function, which I couldn't begin to follow (as an Oracle function) without the comments. That said, there are several things that go back to Pam's point about naming conventions. The function name example.idt_f_get_constant_dt could be more descriptive, and the count is selected into a value named v_count_nbr, which is pretty generic too. But Pam made another point, which is to set up a standard, and stick to it. So if everyone in your shop agrees that dt is the abbreviation to use for date, and the v_ means something to everybody, then that's what's important.

As long as it's all written down somewhere.

Mattie

RE: Writing Maintainable Code

JT Lovell — Mon, 07 Aug 2006 09:11:00 GMT

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 DATEIS 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;

RE: Writing Maintainable Code

CELKO — Mon, 07 Aug 2006 09:06:00 GMT

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.)

RE: Writing Maintainable Code

MattieNH — Mon, 07 Aug 2006 08:37:00 GMT

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

RE: Writing Maintainable Code

Paul Chapman-279652 — Mon, 07 Aug 2006 08:30:00 GMT

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.

RE: Writing Maintainable Code

CELKO — Mon, 07 Aug 2006 08:26:00 GMT

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

RE: Writing Maintainable Code

JT Lovell — Mon, 07 Aug 2006 08:22:00 GMT

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.

RE: Writing Maintainable Code

MattieNH — Mon, 07 Aug 2006 07:22:00 GMT

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

RE: Writing Maintainable Code

Jeff Moden — Mon, 07 Aug 2006 06:13:00 GMT

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

RE: Writing Maintainable Code

David le Quesne — Mon, 07 Aug 2006 01:51:00 GMT

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

Writing Maintainable Code

Pam Brisjar — Thu, 13 Jul 2006 21:29:00 GMT

Comments posted to this topic are about the content posted at http://www.sqlservercentral.com/columnists/pabdulla/writingmaintainablecode.asp