Sunday, 30 June 2024

Fun and games with the :$.PAR parameter / 2

I proudly connect to the client's client's computer, update the IVSTORNO replacement procedure ... and watch it do nothing.

It took a while to realise that I work with the classic, Windows, interface where EXECUTE WINACTIV is fine, whereas the client works with the web interface that requires EXECUTE ACTIVATE. Once that little detail was fixed, the procedure worked flawlessly.

Saturday, 29 June 2024

Fun and games with the :$.PAR parameter

A client of a client wanted, when cancelling an invoice, to add a reason for the cancellation. This would be done by creating a new procedure (actually two) that gets the reason from the user, updates the invoice then calls one of the standard procedures for cancelling invoices (IVSTORNO or IVSTORNO2). I falsely assumed that this wouldn't be much of a problem. First I defined a new table that would hold the reasons for cancellation, then I developed a screen for this table; next I added an appropriate field to the INVOICES table and added the appropriate fields to the forms AINVOICES and CINVOICES. So far so good. As the procedures that cancel invoices are called by direct invocation, they have a :$.PAR parameter that is a linked file holding the number of the invoice to be cancelled.

My original attempt at adding the reason code to the cancelling procedure went as follows

LINK INVOICES TO :$.PAR; GOTO 99 WHERE :RETVAL <= 0; :IV = 0; SELECT IV INTO :IV FROM INVOICES WHERE IV > 0; UNLINK INVOICES; GOTO 99 WHERE :IV = 0; UPDATE INVOICES SET NEWFIELD = :$.NF WHERE IV = :IV; /* call original procedure */ EXECUTE WINACTIV '-P', 'IVSTORNO', 'INVOICES', :$.PAR; LABEL 99;

The first part of this procedure worked perfectly: I extracted the id number of the invoice that was about to be cancelled and updated the reason field. But when I went to cancel it, the cancelling procedure failed. When I removed my code out of this procedure, the cancellation worked properly (thus showing that the 'execute winactiv' call was correct).

I was facing a conundrum: how can I update the invoice when the only way of getting its number is via the :$.PAR procedure? Should I access the parameter, apparently :$.PAR gets emptied and so the call to IVSTORNO will fail. I tried workarounds like saving the :$.PAR parameter in a local variable then using this variable for the IVSTORNO call but this too failed.

After much playing around, I did find a method that didn't involve accessing the :$.PAR parameter. My thinking was that instead of trying to locate the invoice that is to be cancelled, find the invoice that has been created to cancel out the original invoice; this new invoice holds a pointer to the original invoice in the field PIV. So my code went as follows

EXECUTE WINACTIV '-P', 'IVSTORNO', 'INVOICES', :$.PAR; :IV = :PIV = 0; SELECT MAX (IV) INTO :IV FROM INVOICES WHERE TYPE IN ('A', 'C') AND STORNOFLAG = 'Y'; /* this will be the cancelling invoice */ GOTO 99 WHERE :IV = 0; SELECT PIV INTO :PIV /* this is the original invoice */ FROM INVOICES WHERE IV = :IV; GOTO 99 WHERE :IV = 0; UPDATE INVOICES SET NEWFIELD = :$.NF WHERE IV = :IV; LABEL 99;

This actually worked but I wasn't too happy with it as someone else might be cancelling an invoice at the same time. Let's say that it has a 99.99% chance of working as opposed to a 100% chance (i.e. certainty). In other words, a kludge.

An hour or so later, the correct solution popped into my mind

LINK INVOICES TO :$.PAR; GOTO 99 WHERE :RETVAL <= 0; :IV = 0; SELECT IV INTO :IV FROM INVOICES WHERE IV > 0; UNLINK AND REMOVE INVOICES; GOTO 99 WHERE :IV = 0; UPDATE INVOICES SET NEWFIELD = :$.NF WHERE IV = :IV; /* Restore the value in :$.PAR */ LINK INVOICES TO :$.PAR; GOTO 99 WHERE :RETVAL <= 0; INSERT INTO INVOICES SELECT * FROM INVOICES ORIG WHERE IV = :$.IV; UNLINK INVOICES. /* call original procedure */ EXECUTE WINACTIV '-P', 'IVSTORNO', 'INVOICES', :$.PAR; LABEL 99;

I've bolded the new parts of this procedure. The first part of this code is the same as my original code albeit with one significant difference: :$.PAR is unlinked and removed. This basically frees up the parameter for future use. After the invoice is updated with the cancellation code, INVOICES is once again linked to :$.PAR, but now the linked file is empty; the original record is then inserted into the linked file and the file closed. As far as IVSTORNO is concerned, nothing has changed; :$.PAR contains the key to the invoice that must be cancelled.

Tuesday, 18 June 2024

Learning something new every day (GROUP BY without an aggregation function)

I'm sure that we've all written procedures that send a report to all customers that ordered today (it doesn't have to be customers; it can be vendors or users in service calls or similar). I've always done this by means of a cursor such as

SELECT CUSTOMERS.CUSTNAME, COUNT (*) FROM ORDERS, CUSTOMERS WHERE ORDERS.CUST = CUSTOMERS.CUST AND ORDERS.CURDATE = SQL.DATE8 GROUP BY 1;

I saw that someone had done something similar, but instead of using COUNT, he was using DISTINCT and instead of CUSTOMERS.CUSTNAME, a conditional expression was used, something like

SELECT DISTINCT (ORDERITEMS.ICURRENCY <> ORDERS.CURRENCY ? ORDERITEMS.ICURRENCY : ORDERS.CURRENCY), ...

I try to avoid using DISTINCT like the plague; to me it always smacks of lazy programming. One use that I do condone is if I want to know how many separate customers ordered today, as opposed to knowing how many orders each customer made today. I think that the above doesn't work properly because the field that DISTINCT is trying to filter is a conditional field whose value probably isn't known when DISTINCT does its work.

Today I discovered a new twist on the first query shown above: it turns out that the count is unnecessary (assuming of course that I only want a list of individual customers who ordered today). The following gives the desired result and should be faster as there is no need for counting. To my surprise, this syntax works in Priority.

SELECT CUSTOMERS.CUSTNAME FROM ORDERS, CUSTOMERS WHERE ORDERS.CUST = CUSTOMERS.CUST AND ORDERS.CURDATE = SQL.DATE8 GROUP BY 1;

In other words, one can use 'GROUP BY' without an aggregation function (but not the other way around!).

Monday, 17 June 2024

Unprepared forms

There are two rules that one has to know when preparing forms: one is obvious and one is less obvious. The obvious rule is that a form has to be prepared after adding a field or a trigger to it (a form does not have to be prepared if a procedure or report is added for direct activation).

The less obvious rule is that if one adds a table to a form, a table that was not previously defined on that form, then every form that is built on that table has to be prepared.

An example: I was asked to add the status of a purchase order line to the form DOCPORDI ('choose order items', a sub-form of DOCUMENTS_P, 'Goods Receiving Vouchers'). This form is built on the PORDERITEMS table; the order line status description can be found in PORDISTATUSES, and one has to use the PORDERITEMSA table to connect between these two. Thus one would add the following lines to the form

PORDERITEMS.ORDI = PORDERITEMSA.ORDI, identifier 5? PORDERITEMSA.PORDISTATUS = PORDISTATUSES.PORDISTATUS (identifier 5 on both sides) PORDISTATUSES.PORDISTATUSDES to be displayed, identifier 5

First of all, the identifier 5? is used to introduce the PORDERITEMSA table - 5 because it is not an original table of this form, and ? because not every line in the PORDERITEMS table has a corresponding line in PORDERITEMSA (e.g. not every line has a status). The identifier 5 has to be used with PORDISTATUSES because this table too is not original.

Obviously, DOCPORDI has to be prepared, but also any form built on the tables PORDERITEMSA and PORDISTATUSES has to be prepared (i.e. the forms PORDISTATUSES, PORDIORDI and ORDPORD), even though no changes have been made to these forms. If there are many forms that have to be prepared then of course it's best to run the 'prepare all unprepared forms' program.

How does one know specifically which forms have to be prepared? This is a question that I have often wanted to ask, and a few weeks ago I found the answer. A simple SQL query would be

SELECT EXEC.ENAME FROM EXEC, EXECPREPLOCK WHERE EXEC.EXEC = EXECPREPLOCK.EXEC AND EXEC.TYPE = 'F' AND EXECPREPLOCK.UPD <> 'N' FORMAT;

I have built a very simple report based on this query and added it to the scheduler so that I would receive a report every 15 minutes if there is an unprepared form anywhere in the system. I have found that it's better to reprepare any forms that appear in this report as opposed to simply preparing  them. I don't know what the difference between preparing and repreparing is, except that the latter is 'stronger' and takes more time. This always reminds me of a scene in the film 'A few good men', when Demi Moore objects to something in the trial, then strenuously objects. This results in a sarcastic remark from Kevin Pollak (I forget the exact wording); if preparing a form doesn't work then reprepare it.

Wednesday, 12 June 2024

An exercise in how NOT to build private forms

I was recently asked to look at a form that someone (with whom contact has been lost) had defined that displayed purchase order lines belonging to a single vendor; the users wanted to update a private field in the form but were not succeeding in doing so.

I had been asked maybe a month earlier to program for the same customer the same kind of form (purchase order lines of a given vendor) so I wasn't prepared for the mess that I saw. I can understand why the person who programmed this form has disappeared, for it is an excellent example of how not to build a private form and shows many misunderstandings.

Probably the biggest error was not basing the form on PORDERITEMS but instead on a private table. Had the form been based on the standard table then adding a field would have been simple: add the field to the table and then to the screen. But no: this form was based on a private table that of course had a primary key based on PORDERITEMS.ORDI. But not only that: the current user was also part of the primary key! In other words, I could open the form and you could open the form and the possibility exists that we might be seeing different data!

The private field that the customer wanted to be able to update was held in this private table (the person who added the field [not the original programmer] presumably saw on what the table the form was based and so added the field to this table). I didn't see at first that the form was based on this table; I assumed that it was based on PORDERITEMS and so I added a form post-update trigger that would update the private table if the private field were changed. This didn't work. Changes in the field were maintained as long as the form was displayed on the screen, but would vanish when the form was reopened. Eventually I found the reason for this: the form has a PRE-FORM trigger that first empties the private table for the current user then enters the appropriate values for PORDERITEMS.ORDI. 

Excuse me????? What is this rubbish? Such a convoluted structure for something that should be far simpler. In the end, in order to satisfy the customer, I added the required field to PORDERITEMS and updated this (by placing the table/column name combination in the field continuation) instead of using the same field in the private table.

A further request for modifying the existing form was to add the connected customer order if one exists. I saw that already there was some connection to ORDERITEMS so I didn't need to add this; I added the appropriate fields and ran the form. The form would not open. I removed the fields that I had added and reopened the form - it worked properly. I then took a much closer look at the form definitions and saw that ORDERITEMS had been defined with the identifier (alias) 5! (the exclamation mark was not part of the identifier). This is correct if one is adding fields to a table in a standard form, but is totally unnecessary if one is doing this in a private form. This is a clear example of someone not understanding the SDK. Naturally the field that I had added did not have this identifier; adding it allowed the form to be displayed.

Another problem with this form became clear when the customer explained that users can change the status of the connected purchase order from within this form (that's why they thought that the private field that they added was capable of being updated from this form). Maybe the code for this was correct - I didn't waste much time in looking at it - but it was clear that the most basic check had been ignored. First check that the field holding the status had been modified and only then update the order status! But no, this trigger blindly modified the status over and over again (if I remember correctly, this abomination involved touching the table several times, a complete waste of time). Any kind of trigger like this should start as follows

GOTO 99 WHERE :$1.<FIELD> <> :$.<FIELD> ; /* update field */ LABEL 99;

If at the beginning of my examination I wanted to put a gun to the head of the person that programmed this atrocity, by the end I wanted to put a machine gun there. This sort of programming gives independent programmers a bad reputation.
 
So what are the lessons to be learnt?
  1. If one wants to display data from an existing table, it's best to base the private form on that table and define the form either as Q (read only) or N (no deletions). One can add private forms either to the standard table or to a private continuation form; doing the former 'contaminates' the standard table (although this is condoned) whereas the latter requires programming a trigger to update the private table.
  2. There is no need to use the 5 identifier when one is working on a private form; this is required only when adding new tables to a standard form (or report).
  3. Check whether a field has been modified first before writing code to update its value in the database.