Sunday, 25 August 2024

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

No comments:

Post a Comment