Jump to content

Questions about best practices for code & beatifying code


 Share

Recommended Posts

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 by Zoey Mattison
Link to comment
Share on other sites

  • Zoey Mattison changed the title to Questions about best practices for code & beatifying code

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

 

Link to comment
Share on other sites

  • 2 months later...

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:
image.png.7bab53696e4d6843f6ed48ce232d2dea.png

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

image.thumb.png.5aa68ed3c922342f5cb1cde0293a7bc6.png

Link to comment
Share on other sites

  • 3 weeks later...
Posted (edited)

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 by Zoey Mattison
Link to comment
Share on other sites

Please sign in to comment

You will be able to leave a comment after signing in



Sign In Now
 Share

×
×
  • Create New...