Using NFILE prevents complications

Someone sent me some code written by a third party that inserts data into the table LABELS, naturally for printing labels. The person who sent me the code wanted the sort order of the labels to be changed so that it would be according the part numbers sorted alphabetically. As the original programmer used the INSERT INTO/SELECT FROM syntax, it wasn't possible to simply add 'ORDER BY PART.PARTNAME' at the end of the query. 

The solution was provided by what seemed to be a somewhat pointless subroutine earlier in the program.

SUB 20;
INSERT INTO STACK4 (KEY)
SELECT PART
FROM   PART
WHERE  PART <> 0;
RETURN;

Saving the parts in this table gave me the opportunity to add a sort order to STACK4 as per the following.

:SORDER = 0;
DECLARE CSUB50 CURSOR FOR
SELECT PART, PARTNAME
FROM PART
WHERE PART > 0
ORDER BY PARTNAME;
OPEN CSUB50;
GOTO 530 WHERE :RETVAL <= 0;
LABEL 510;
FETCH CSUB50 INTO :PART, :PNAME;
GOTO 520 WHERE :RETVAL  <= 0;
:SORDER = :SORDER + 1;
UPDATE STACK4
SET INTDATA = :SORDER
WHERE KEY = :PART;
LOOP 510;
LABEL 520;
CLOSE CSUB50;
LABEL 530;

Then I could make use of STACK4.INTDATA in the statement that inserts the data into LABELS, ensuring that the labels would indeed be sorted by partname. Later on I noticed that the programmer had not used the field LABELS.SORT - STACK4.INTDATA can be inserted into this field in order to ensure the correct sort order [see below *].

Reviewing the code. I saw that the programmer had declared a subroutine 30 that on first glance appeared to be the same as subroutine 20; the only difference was that instead of FROM PART, the second subroutine had FROM PART ORIG. In other words, using the unlinked PART table instead of the linked table. How was the programmer checking whether PART was linked? By the following code

:E_COUNT = 0;
SELECT COUNT(*)
INTO   :E_COUNT
FROM   PART
WHERE  PARTNAME NOT IN ('', :E_CHVAL)
;
:E_ORIGCOUNT = 0;
SELECT COUNT(*)
INTO   :E_ORIGCOUNT
FROM   PART ORIGPART
WHERE  PARTNAME NOT IN ('', :E_CHVAL)
;
:E_LINKPART =
( :E_COUNT <> :E_ORIGCOUNT AND :E_COUNT <> 0 ? 'Y' : '\0' )

Either SUB 20 or SUB 30 would be invoked, depending on the value of the variable :E_LINKPART . Although I haven't seen the definitions of the various parameters,I am sure that the parameter PAR (implying that this code is called from another procedure) is defined as FILE. If it were defined as NFILE, then there is no need for :E_LINKPART and only one subroutine would be needed.

LINK PART TO :$.PAR;
ERRMSG 500 WHERE :RETVAL <= 0;
GOTO 1 FROM PART WHERE PART > 0;
UNLINK PART;
LABEL 1;
...
GOSUB 20;

I realise that I've made a mountain out of a molehill, but it seems that the original programmer had to build a baroque solution to solve a problem that has a very simple solution. This is not the first time I've seen this, and I think that it stems from an incomplete understanding of how to program - not necessarily how to program in Priority.

Even without using NFILE, the programmer still could have saved the SUB 20/30 duplication:

:E_COUNT = :E_ORIGCOUNT = 0;
SELECT COUNT (*) INTO :E_COUNT FROM PART;
SELECT COUNT (*) INTO :E_ORIGCOUNT FROM PART ORIG;
GOTO 1 WHERE :E_COUNT <> :E_ORIGCOUNT;
UNLINK PART;
LABEL 1;

After all, there's no point in the clause WHERE PARTNAME NOT IN ('', :E_CHVAL) if the same clause is used in both queries; at worst, both values might be one too high (e.g. 37 instead of 36) but as equality is being checked, it makes no difference. Incidentally, :E_CHVAL appears not to be defined anywhere in the procedure; another rookie mistake (it turns out that :E_CHVAL was defined in a previous stage of the procedure).

* To show that I too am not immune to writing sub-optimal code, the code that enters data into STACK4 could be rewritten as below, without the need of a prior INSERT INTO statement. There's no real need to use a subroutine at all as this code is called only once, although I understand why the programmer did this: using subroutines gives the procedure a sense of structure.

SUB 20; /* Insert data into stack4 in partname order */
DECLARE CSUB20 CURSOR FOR
SELECT PART, PARTNAME, SQL.LINE
FROM PART
WHERE PART > 0
ORDER BY PARTNAME;
OPEN CSUB20;
GOTO 230 WHERE :RETVAL <= 0;
LABEL 210;
FETCH CSUB20 INTO :PART, :PNAME, :LINE;
GOTO 220 WHERE :RETVAL  <= 0;
INSERT INTO STACK4 (KEY, INTDATA)
VALUES (:PART, :LINE);
LOOP 210;
LABEL 220;
CLOSE CSUB20;
LABEL 230;
RETURN;