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)
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.
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;
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.
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.
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.
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!
Obvious, from reading code?
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)
ReadyStatus = ( ReadyStatus = False )
So should this be documented, or rewritten?
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.