Questions about best practices for code & beatifying code

[Zoey Mattison]

Hi everyone,

I've written some rather cursed code and was wondering if anyone could help me improve or just beautify how it looks. I am not really privy to that side of sage's programming language and previous attempt result in syntax error so I am seeking any assistance you can give. As it is now, the below code functions flawlessly and does not seem to impact load times of GESPAY documents.

This code I've written pulls some extraneous data from certain tables, and places it into a custom field in the GESPAY function screen PAY1 array. There are 3 conditions it checks in order to run, and when I tried to convert it to a case, it doesn't like when I use the pat() function (incorrect data type), so I've gone with three separate if functions here. The code snipped is called via STYLE when a document is opened.

$ZOKYAKU
If !Clalev([ZPI]) : Local File PINVOICED[ZPI] : Endif

For i = 0 to [M:PAY1]NBLIG-1

If (pat([M:PAY1]VCRNUM(i), 'ORD*') | pat([M:PAY1]VCRNUM(i), 'BTS*') | pat([M:PAY1]VCRNUM(i), 'WEB*'))
 [M:PAY1]ZOKYAKU(i) = func AFNC.INTFLD('SORDER','BPCINV',[M:PAY1]VCRNUM(i)+'~')+' - '+func AFNC.INTFLD('SORDER','BPINAM',[M:PAY1]VCRNUM(i)+'~')
Endif

If (pat([M:PAY1]VCRNUM(i), 'PIN*') | pat([M:PAY1]VCRNUM(i), 'PCM*'))
 Filter [ZPI] Where NUM = [M:PAY1]VCRNUM(i)
 Read [ZPI] first
 [M:PAY1]ZOKYAKU(i) = func AFNC.INTFLD('BPSUPPLIER','BPSGRU',func AFNC.INTFLD('PRECEIPT','BPSNUM',[ZPI]NUMORI))+' - '+func AFNC.INTFLD('BPSUPPLIER','BPSNAM',func AFNC.INTFLD('PRECEIPT','BPSNUM',[ZPI]NUMORI))
Endif

If (pat([M:PAY1]VCRNUM(i), 'DIR*') | pat([M:PAY1]VCRNUM(i), 'INV*') | pat([M:PAY1]VCRNUM(i), 'INT*') | pat([M:PAY1]VCRNUM(i), 'SCM*'))
 [M:PAY1]ZOKYAKU(i) = func AFNC.INTFLD('SINVOICE','BPR',[M:PAY1]VCRNUM(i)+'~')+' - '+func AFNC.INTFLD('SINVOICE','BPRNAM',[M:PAY1]VCRNUM(i)+'~')
Endif

Affzo [M:PAY1]ZOKYAKU(i)
Next
Return

Thanks, and have a great day

Zoey

Edited January 9, 2024 by Zoey Mattison

[Zoey Mattison]

I've re-written some of the calls to use GACCENTRYD and accounted for InfoPOS documents mostly not using PAY1 mask (error: NBLIG non existente)

If !Clalev([ZPI]) : Local File PINVOICED[ZPI] : Endif
If !Clalev([ZGA]) : Local File GACCENTRYD[ZGA] : Endif
If !pat([M:PAY0]NUM, 'IP*') 
For i = 0 to [M:PAY1]NBLIG-1
 If (!pat([M:PAY1]VCRNUM(i), 'ORD*') & !pat([M:PAY1]VCRNUM(i), 'BTS*') & !pat([M:PAY1]VCRNUM(i), 'WEB*'))
  Filter [ZGA] Where TYP = [M:PAY1]VCRTYP(i) & NUM = [M:PAY1]VCRNUM(i)
   Read [ZGA] First
    If fstat = 0
     If [ZGA]BPR <> 'V5555'
      [M:PAY1]ZOKYAKU(i) = [ZGA]BPR + ' - ' + func AFNC.INTFLD('BPARTNER','BPRNAM',[ZGA]BPR)
      Affzo [M:PAY1]ZOKYAKU(i)
     Else
      Filter [ZPI] Where NUM = [M:PAY1]VCRNUM(i) & NUMORI <> ''
       Read [ZPI] first
        If fstat = 0
         [M:PAY1]ZOKYAKU(i) = func AFNC.INTFLD('PRECEIPT','BPSNUM',[ZPI]NUMORI)+' - '+func AFNC.INTFLD('BPARTNER','BPRNAM',func AFNC.INTFLD('PRECEIPT','BPSNUM',[ZPI]NUMORI))
        Affzo [M:PAY1]ZOKYAKU(i)
        Endif
     Endif 
    Endif
 Else
  [M:PAY1]ZOKYAKU(i) = func AFNC.INTFLD('SORDER','BPCINV',[M:PAY1]VCRNUM(i)+'~')+' - '+func AFNC.INTFLD('SORDER','BPINAM',[M:PAY1]VCRNUM(i)+'~')
  Affzo [M:PAY1]ZOKYAKU(i)
 Endif
Next
Endif

 

[Gonzalez, Bruno]

Hi Zoey,
It seems you aren't using Eclipse. I would recommend you to do so. https://plugin-x3.sagex3.com/safex3/studio/downloads/index.html

Every developer has its own style, but please find some common rules we share with many developers in my team (we try at least 😄). I voluntarily stay out of the pat instruction topics or how to code for best performance is it a different topic and an actual training that exists with Sage. I will however say thins: look at Columns and Columns Extended instructions, you will thank me later 😎

https://online-help.sagex3.com/erp/12/en-us/Content/V7DEV/4gl_columns.html?Highlight=columns

- Don't make your life more  difficult that it already is: your code should be the right balance between readability and maintenance/Search.

- Indent with tab key

- Indent can be 2 or 4 spaces. Don't mind, don't care.

- Separate clearly "features". Like table opening VS actual business code

- No spaces around operators: +, =, <>,-

- Spaces after a comma separating parameters - this one is mine only, but it helps me see clearer parameters list and also helps with searches

- Always specify classes of variables: [F], [M], [V],[S],[L].  Don't leave any doubts possible on the nature of the variable. It also helps the X3 engine avoiding to scan the 5000 variables that exists. FYI Sage X3 studio plugin in Eclipse autocompletes most of the classes, except the F class where you need to explicitly specify it so that Eclipse can list the table fields for you with its details:

- Always double double-quotes to manage text.

- Keep simple quotes when you need to manage complex strings using double-quotes inside

- Always Specify the element you are looping over in a For Next loop on the Next clause, like Next I, or Next ITM - please note when looping over a table, just give the table abbreviation

- Split complex or long lines of code  by using the & sign on the new line

- Avoid over-indenting things to align pad everything vertically. It might look nicer but it makes searches more difficult as you need to use regex to remove extra spaces.

- ALWAYS CLOSE YOUR FILTER INSTRUCTION, CAN KEEP THE CLOSING FILTER VISIBLE TO THE OPENING FILTER ON A SINGLE PAGE. Sorry about this one, but this is a typical mistake and it is very very very (did I emphasized on very ?) difficult to find afterwards.

- I don't like using & and | signs because they are not colored in Sage X3 Studio plugin of Eclipse, so I use and and or in plan text

- Put code snippets that you repeat often in label, and use gosub

- create functions every time you can

Please find below how I would have done it:
 

If !clalev([F:ZPI]) : Local File PINVOICED[F:ZPI] : Endif
If !clalev([F:ZGA]) : Local File GACCENTRYD[F:ZGA] : Endif

If !pat([M:PAY0]NUM, "IP*")
  For I=0 To [M:PAY1]NBLIG-1
    If (!pat([M:PAY1]VCRNUM(I), "ORD*")
&     and !pat([M:PAY1]VCRNUM(I), "BTS*")
&     and !pat([M:PAY1]VCRNUM(I), "WEB*"))
      Filter [F:ZGA] Where [F:ZGA]TYP=[M:PAY1]VCRTYP(I) and [F:ZGA]NUM=[M:PAY1]VCRNUM(I)
      Read [F:ZGA] First
      If [S]fstat=0
        If [F:ZGA]BPR<>"V5555"
          [M:PAY1]ZOKYAKU(I)=[F:ZGA]BPR+" - "+func AFNC.INTFLD("BPARTNER", "BPRNAM", [F:ZGA]BPR)
          Gosub $AFFZO_ZOKYAKU
        Else
          Filter [F:ZPI] Where [F:ZGA]NUM=[M:PAY1]VCRNUM(I) and [F:ZGA]NUMORI<>""
          Read [F:ZPI] First
          If [S]fstat=0
            [M:PAY1]ZOKYAKU(I)=func AFNC.INTFLD("PRECEIPT", "BPSNUM", [F:ZPI]NUMORI)+" - "+
&                             func AFNC.INTFLD("BPARTNER", "BPRNAM", func AFNC.INTFLD("PRECEIPT", "BPSNUM", [F:ZPI]NUMORI))
            Gosub $AFFZO_ZOKYAKU
          Endif
          Filter [F:ZPI]
        Endif
      Endif
      Filter [F:ZGA] #Super important !
    Else
      [M:PAY1]ZOKYAKU(I)=func AFNC.INTFLD("SORDER", "BPCINV", [M:PAY1]VCRNUM(I)+"~")+" - "+
&                       func AFNC.INTFLD("SORDER", "BPINAM", [M:PAY1]VCRNUM(I)+"~")
      Gosub $AFFZO_ZOKYAKU
    Endif
  Next I
Endif

End

$AFFZO_ZOKYAKU
  Affzo [M:PAY1]ZOKYAKU(I)
Return

And what it looks like in Eclipse:

[Zoey Mattison]

Hi @Gonzalez, Bruno! Thanks for all of your input. I really appreciate the instruction about formatting. I just have a few questions:

1. What is the importance of adding the Filter [CLASS] at the end of the block requiring the filter? (Ie. Filter [F:ZPI] & Filter [F:ZGA] as you mentioned on line 24 and 27?)
2. What are classes [V], [S], and [L]? I've never come across that before.
   • Based on that note you wrote about always including them, our own consultant randomly doesn't in their code, so lately I have been writing everything like that without declaring the class (except for masks) when everything is contained within the file. Should I add the F to all of these below? Would the processing time decrease?

Here is an example of a program I wrote for a recurring task that looks for documents in PAYMENTH with a NUM field pattern like PAYET* and which as posted, and sends a custom crystal report I made (ZPAYET) to their email address stored in ZPAYET_EMAIL via a workflow where one of the conditions is that ZID = 'INTEGRATION' to make doubly sure it cannot be executed manually via the report screen accidentally . The PAYET number is then stored in a custom database table called ZDOCDATA which acts as as a filter for the program to know whether or not a PAYET document has already been sent to the customer (!find(NUM, ZMAILED)).

Our server does not have eclipse debugging enabled, so while I've tried to set it up before, it never works. I use VS code with the Sage X3 plugin instead.

I'll update everything in here with the formatting you mentioned, but would like some clarification about the inclusion of classes ([F], etc.) and the adding a filter with no where clause at the end. I am not trained or anything, just learning via example and online help so anything you can provide with regards to making the code better which as much detail as possible would be highly appreciated! I redacted some information below for privacy reasons. Thanks!

If !Clalev([ZDD]) : Local File ZDOCDATA[ZDD] : Endif
If !Clalev([ZPY]) : Local File PAYMENTH[ZPY] : Endif

Global Clbfile ZPAYET_EMAIL
Global Char ZID
    ZID = 'INTEGRATION'
Local Char TBPAR_BP(15)(1..50),TBVAL_BP(30)(1..50)
    TBPAR_BP(1) = "regdeb"
Local Char ZMAILED(50)(1..)
Local Integer ZI
    ZI = 1

Filter [ZDD] Where ZTYPE = 'PAYET' and ZNAME = 'MAILING' And CREDAT >= date$-100

For [ZDD]
    ZMAILED(ZI) = [ZDD]ZNUM
    ZI = ZI+1
Next

Filter [ZPY] Where pat(NUM, 'PAYET*') and !find(NUM, ZMAILED) and STA = 9 and CREDAT >= date$-90

For [ZPY]
    ZPAYET_EMAIL = func AFNC.INTFLD('BPSUPPLIER','YACHWEB',[ZPY]BPR)
    if ZPAYET_EMAIL = '' : ZPAYET_EMAIL = '[email protected]' : Endif
    TBVAL_BP(1) = [ZPY]NUM
    [ZDD]ZNUM = [ZPY]NUM
    [ZDD]ZTYPE = 'PAYET'
    [ZDD]ZNAME = 'MAILING'
    [ZDD]CREDAT = [ZPY]CREDAT
    Write [ZDD]
    Call ETAT("ZPAYET","EMAILX3","",0,"",TBPAR_BP,TBVAL_BP) From AIMP3
Next

Raz ZPAYET_EMAIL, ZID, TBPAR_BP, TBVAL_BP, ZMAILED, ZI

Close Local File [ZDD], [ZPY]
End

 

Edited April 17, 2024 by Zoey Mattison

[Gonzalez, Bruno]

Hi @Zoey Mattison
1 - When you use the filter instruction, you build the where clause of the SQL query. Not closing the filter clause means that you keep the existing filters. Let's say later you do a filter again, you will then accumulate the former filter plus the new one.

This is a typical mistake where you will miss records and you don't understand why. To make it simple, always close filter instruction and columns instruction.

2- [L], [S],[F],[M],[V] are "classes" of variables in memory. You can use Eclipse debugger to see the list of variable for each class:

• L means local variables:
  local char MYVAR (35) : [L]MYVAR="test"
• S means system variables: you don't create them, you use the ones provided by X3, like fstat.
• F means table
• M means screen
• V means global variables: they have a scope of life which covers everything: you create it and it remains available everywhere (subprogs, funprogs, etc.). Don't forget to kill them eventually and manage their value properly
• C means chronological counter: using table APLCOM to provide a sequence number automatically. This is old school and almost not used, but this is basically what is used until 2024R2 to generate the trace file number FNNNNN.tra files

There are others, but it's a good start 🙂

Adding the class helps X3 knowing what it is without having to scan for the object in the memory stack, and avoids silly mistakes as you can have the same name used for a screen field and a variable like

Niveau+=1

it could [M]Niveau to be a screen field name, or [L] niveau to be a local variable.

It is recommended to always be specific with variable classes, but even at Sage we sometimes forget. Eclipse is helpful as it can detect the class when you hit ctrl+space when you write code.

3 - Eclipse and debugger does work, but there are conditions for it to work. You need port 9514 to be opened between X3 runtime and Syracuse and potentially attach the process you want to debug manually if the test user is different from the dev user:

4- there is no VSCode plugin, simply a syntax highlighting, so it's the same as using notepad++ with syntax highlight. it means you need to manually compile your code in X3 and you cannot debug.

I would encourage you trying to get trained either by us (Sage) or your partner, because even just a 2 or 3 days advanced training will make a big difference on your understanding, productivity and capacity to produce quality code.

I've rewritten your example to give you a reference, although we could go further for performance reasons

If !clalev([F:ZDD]) : Local File ZDOCDATA [F:ZDD] : Endif
If !clalev([F:ZPY]) : Local File PAYMENTH [F:ZPY] : Endif

Global Clbfile ZPAYET_EMAIL
Global Char ZID
    [V]ZID = 'INTEGRATION'
    #=> Do you really need global variables ?
Local Char TBPAR_BP(15)(1..50),TBVAL_BP(30)(1..50)
    [L]TBPAR_BP(1) = "regdeb"
Local Char ZMAILED(50)(1..)
Local Integer ZI
    [L]ZI = 1

Filter [F:ZDD] Where [F:ZDD]ZTYPE = 'PAYET' and [F:ZDD]ZNAME = 'MAILING' and [F:ZDD]CREDAT >= date$-100
For [F:ZDD]
    ZMAILED(ZI) = [ZDD]ZNUM
    ZI = ZI+1
Next ZDD
Filter [ZDD]

#You need to open a database transaction to be able to rollback and manage errors
Trbegin [F:ZDD]
Filter [F:ZPY] Where pat([F:ZPY]NUM, 'PAYET*') and !find([F:ZPY]NUM, ZMAILED) and [F:ZPY]STA = 9 and [F:ZPY]CREDAT >= date$-90
For [ZPY]
    [V]ZPAYET_EMAIL = func AFNC.INTFLD('BPSUPPLIER','YACHWEB',[F:ZPY]BPR)
    If [V]ZPAYET_EMAIL = '' : [V]ZPAYET_EMAIL = '[email protected]' : Endif
    [L]TBVAL_BP(1) = [F:ZPY]NUM
    [F:ZDD]ZNUM = [F:ZPY]NUM
    [F:ZDD]ZTYPE = 'PAYET'
    [F:ZDD]ZNAME = 'MAILING'
    [F:ZDD]CREDAT = [F:ZPY]CREDAT
    Write [F:ZDD]
#   You need to test fstat to commit or rollback
    If [S]fstat=0
      Commit
      Call ETAT("ZPAYET","EMAILX3","",0,"",[L]TBPAR_BP,[L]TBVAL_BP) From AIMP3
    Else
      Rollback
      Call ECR_TRACE ("Can't write record in ZDD:"-[F:ZPY]NUM,1) From GESECRAN
    Endif

Next ZPY
Filter [F:ZPY]

Raz [V]ZPAYET_EMAIL, [V]ZID, [L]TBPAR_BP, [L]TBVAL_BP, [L]ZMAILED, [L]ZI

Close Local File [ZDD], [ZPY] #Can use LogicClose for better performance if you really want to close it. It's closed automatically anyway because you are in a subprogram.
End