Is this the lamest code I've ever seen?

In the course of debugging a procedure that someone else had written years ago for a customer, I came across the following code:

DECLARE UPD_DATA9 CURSOR FOR                                        
SELECT DTOA (:$.DAT, 'DD/MM/YY') FROM DUMMY;                         
OPEN UPD_DATA9;                                                     
LABEL 10;                                                           
FETCH UPD_DATA9  INTO :IVDATE;                                      
GOTO 100 WHERE :RETVAL <= 0;                                        
UPDATE TEST_INVLOADDIV                                               
SET TEST_IVDATE = ATOD (:IVDATE, 'DD/MM/YY');                         
LOOP 10;                                                            
LABEL 100;                                                          
CLOSE UPD_DATA9;  

This code simply updates the field 'TEST_IVDATE' in all the rows of the table 'TEST_INVLOADDIV' with the date :$.DAT. This must be the lamest code that I've ever seen as there are so many stylistic errors. That is not to say that this code won't achieve what it's supposed to but it is so wrong!

Where should I start? The global parameter :$.DAT is guaranteed to be a date so there's no need to turn it into a string in the second line, then turn it back into a date in the eighth line. There is absolutely no reason to use a cursor that iterates over ... a global parameter! Totally pointless. The above code was written as a separate SQLI stage in a procedure; there's no real reason why it couldn't have been included in another stage with more (bad) code. The above code can be written succinctly as

UPDATE TEST_INVLOADDIV SET TEST_IVDATE = :$.DAT;

Of course, if the programmer were being paid by lines written, then the original version is much better, having 11 lines. My streamlined version wouldn't earn the programmer very much, having only one line. But if the programmer is being paid by her level of sophistication ....

More fixing a complicated problem with BOM revisions and 'info only' parts

The code given in the previous blog was almost correct, but subtly wrong. I had ignored the fact that each son part would appear three times, and that the first instance would set SONACT to be -1 thus causing the son not to appear in the report, even though there would be a correct instance later on. A more correct version of this code is

 SUB 820;
 :OK = 0; /* the query below may fail */
 :RVDATE = 01/01/88;
 SELECT MAX (ORIG.RVTILLDATE) INTO :RVDATE
 FROM PARTARC ORIG
 WHERE ORIG.PART = :PARENT
 AND ORIG.SON = :SON
 AND ORIG.INFOONLY <> 'Y';
 :OK = (:RVDATE = 01/01/50 ? 1 : 0);
 GOTO 821 WHERE :OK = 1;
 UPDATE PARTARC /* effectively remove the part from the tree */
 SET SONACT = -1
 WHERE SON = :SON;
 LABEL 821;
 RETURN;

Saving the maximum value of RVTILLDATE into a variable was not in my original code; this was intended to help with running the procedure with the debugger. But for some reason the 'wonderful' web interface didn't work so I didn't get an automatic debug output.

There was a strange phenomenon whose source I failed to track down, even after wasting an hour on it. The part that I was using for testing should have four active sons, but for some reason five were being displayed. That fifth part had been marked 'for info only' in all the versions so the 'select max' query above should have failed. But somehow this fifth part never even got to the subroutine, a failure that I could not track. Eventually I added a cursor that is called immediately after the SONRAW procedure.

 DECLARE C840 CURSOR FOR
 SELECT SON, SONPARTREV
 FROM PARTARC
 WHERE SON > 0;
 OPEN C840;
 GOTO 843 WHERE :RETVAL <= 0;
 LABEL 841;
 FETCH C840 INTO :SON, :PARENT;
 GOTO 842 WHERE :RETVAL <= 0;
 :A = :B = 0;
 SELECT COUNT (*), SUM ((INFOONLY = 'Y' ? 1 : 0)) INTO :A, :B
 FROM PARTARC PA
 WHERE PART = :PARENT
 AND SON = :SON;
 LOOP 841 WHERE :A <> :B;
 UPDATE PARTARC /* this is the linked table */
 SET SONACT = -1
 WHERE SON = :SON;
 LOOP 841;
 LABEL 842;
 CLOSE C840;
 LABEL 843;

This cursor got rid of the spurious part and probably helped the rest of the procedure run a bit faster as there would have been fewer parts to check in the main cursor.

Working with BOM revisions is very complicated!

Fixing a complicated problem with BOM revisions and 'info only' parts

For a customer, I wrote a year ago a procedure and report that explodes a part's BOM and calculates various data for the part's sons. I thought that the report was working fine until the customer brought to my attention a specific part: despite the fact that its sons appear as "for info only" (FIO) in the BOM table, these son parts were appearing in the report.

How I reached the solution for the next step is lost to me, so I'll say that by divine intervention I saw that the father part had three BOM revisions, and that the sons that were marked FIO in the BOM table were not marked as such in one of these revisions.

The procedure uses the SONRAW program to explode the BOM, where the fourth parameter is 0, i.e. don't include parts marked as FIO. How is it that these parts still appear? Could it be that SONRAW is working on all the BOM revisions and so is finding the revision in which the parts are not marked as FIO?

If one dumps the data contained in the table PARTARC for the given father part, one will see that the son parts appear three times, once for each revision. The difference between these apparently identical lines, apart from the FIO flag, is that their RVFROMDATE and RVTILLDATE fields differ; these values are identical to the revisions in the BOM revisions table of the father.

Whilst walking the dog after the consultation, I thought about this problem and concluded that first I would have to find the line with the maximum value for RVFROMDATE then check whether this line is marked as FIO. It seems that the right hemisphere of my brain was working on this problem all night because at about 2:30 am, I suddenly realised that I don't have to look for a maximum value - I simply need to find the line with RVTILLDATE = 01/01/50.

This morning I came in fired up with enthusiasm and added this condition to the cursor that iterates over the sons. Nothing changed. It turns out that SONRAW zeroes out this field in the linked PARTARC table: annoying. I tried another track: I know that PARTARC.SONPARTREV contains the internal part number of the direct parent. Via this field I could access the original, unlinked, tuples of PARTARC in order to find the appropriate line. As the check has to occur twice in the procedure, I wrote a subroutine that contains the code that checks. 

The part at the end of the subroutine that sets SONACT to be -1 is because the linked PARTARC table is passed to the report that 'knows' not to display lines where SONACT = -1. This was something that eluded me until late in the day. The variable :PARENT is PARTARC.SONPARTREV. The returned variable :OK will cause the procedure to skip parts of the code if the value is 0.

/**************************************************************
 SUB 820 - Check whether son is in the current version
 of the BOM and if so, is info only
 ***************************************************************/
 SUB 820;
 :OK = 1;
 SELECT 0 INTO :OK FROM PARTARC ORIG
 WHERE ORIG.PART = :PARENT
 AND ORIG.SON = :SON
 AND (ORIG.INFOONLY = 'Y'
 OR ORIG.RVTILLDATE <> 01/01/50);
 GOTO 821 WHERE :OK = 1;
 UPDATE PARTARC /* effectively remove the part from the tree */
 SET SONACT = -1
 WHERE PART = :FATHER
 AND SON = :SON;
 LABEL 821;
 RETURN;