Clean Code: Applying in the AdvPL

Today, we will talk about Clean Code techniques, focusing on the AdvPL programming language. For those who don't know, AdvPL is a programming language for the TOTVS company and, on the contrary everyone thinks she can also adopt the Clean Code utilization without problems.

We know the AdvPL has the characteristic of a limitation of characters (the compiler will read only the first 10 letters, if you have more, it won't give an error) in relation to the nomenclature of variables and functions. However, the goal of this article is we address the Clean Code use I leave here some tips which I have followed during my development.

Nomenclature

In this case, we should not look at the limitations that the language imposes on us, but rather know how to abbreviate or write intelligently.

💡 Tips: All variables in use in the code NEED be declare

User Function TEST01()
    Local cOrder      := "" //BAD ❌
    Local cSalesOrder := "" //GOOD ✅

Return

//BAD ❌
Static Function exists()
    //Your code here..

Return

//GOOD ✅
Static Function clientExists()
    //Your code here..

Return

See that in the example above we exceed the max limit, but we agree which legibility of functions and variables name it was left better than abbreviated?

Functions

Parameters

We say in the Clean Code, that a function can't have a lot of parameters, because the more parameters greater than possibility that function to make something that could not.

//BAD ❌
Static Function findClient( cClient, cStore, cOrderClient, cShippingCompany )
    //Your code here...

Return

//GOOD ✅
Static Function findClient( cClient, cStore )
    //Your code here...

Return

Separation of Responsibilities

//BAD ❌
Static Function blockDefaultingClient(cClient, cStore)
    dbSelectArea("SA1")
    SA1->( dbSetOrder(1) )
    SA1->( dbGoTop() )

    If SA1->(msSeek(FWxFilial("SA1") + cClient + cStore))
        RecLock("SA1", .F.)
            SA1->A1_MSBLQL := "1"
        SA1->( msUnlock() )
    EndIf

Return

//GOOD ✅
Static Function processBlockDefaultingClient(cClient, cStore)
    dbSelectArea("SA1")
    SA1->(dbSetOrder(1))
    SA1->(dbGoTop())

    If isValidClient(cClient, cStore)
        //This function can be reused in another file
        blockDefaultingClient(cClient, cStore)
    EndIf

Return

/*
    Function responsible to block any defaulting (late payment) client using
*/
Static Function blockDefaultingClient(cClient, cStore)

    RecLock("SA1", .F.)
        SA1->A1_MSBLQL := "1"
    SA1->( msUnlock() )

Return

/*
    Function responsible to verified if client is valid
*/
Static Function isValidClient(cClient, cStore)
    Local lIsValidClient := .F.

    If SA1->(msSeek(FWxFilial("SA1") + cCodCli + cStore))
        lIsValidClient := .T.
    EndIf

Return lIsValidClient

Comments

A good function needs to be self-explanatory, i.e, you don't need a lot of comments. Remember, if a function has a lot of comments, it's time to review it. Comments lie!!!!

//BAD ❌
Static Function blockDefaultingClient(cClient, cStore)

    //Valid if cliente exists
    If validClient() 
        //**WRONG**...need to valid if is defaulting client 

        //More comments
        //More code..
    EndIf

Return

//GOOD ✅
/*
    Function responsible to make anything....
*/
Static Function blockDefaultingClient(cClient, cStore)
    If validDefaultingClient()
        //More code..
    EndIf

Return

Use of Return

Just like all in the life has a begin and end, with code is not different. Prefer use logical variables to control flux than use Return in the middle code

//BAD ❌
Static Function saveClient( cClient, cStore )

    dbSelectArea("SA1")
    SA1->( dbSetOrder(1) )
    SA1->( dbGoTop() )

    If SA1->(msSeek(FWxFilial("SA1") + cClient + cStore))
        Return
    EndIf

    //Here never be executed
    //More code...

Return

//GOOD ✅ 
Static Function saveClient( cClient, cStore )
    Local lClientFound := .F.

    dbSelectArea("SA1")
    SA1->(dbSetOrder(1))
    SA1->(dbGoTop())

    If SA1->(msSeek(FWxFilial("SA1") + cClient + cStore))
        lClientFound := .T.
    EndIf

    //Here ever be executed
    If lClientFound
        //More code...
    EndIf

    SA1->( dbCloseArea() )

Return

Avoid Magic Numbers

​Just as a spectator is not required to know how a magic number is made, you are also not required to waste hours of analysis identifying what a value represents. When using fixed information, avoid “throwing” it directly into the source, prefer the use of constants with a suggestive name, making it easier for another developer to analyze. Note below the difference in the interpretation of a source with a well-represented value to a source with a simply stated value.

//BAD ❌
User Function calculateBonus(cEmployer, nSalary)
    Local lClienteEncontrado    := .T.

    nSalario += 100
    nSalario *= 2

    If nSalary > 1000
        nSalary := nSalary + (0.1 * nSalary)
    EndIf

Return

//GOOD ✅
#Define MIN_SALARY_TO_BONUS 1000

User Function calculateBonus(cEmployer, nSalary)
    Local lClienteEncontrado    := .T.

    If nSalary > MIN_SALARY_TO_BONUS
        nSalary := nSalary + (0.1 * nSalary)
    EndIf

Return

Else use

Excessive use of Else can also bring unnecessary complexity to the code.

//BAD ❌
Static Function validateClient()
    Local lClientFound := .F.

    If findClient()
        lClientFound := .T.
    Else
        lClientFound := .F. //This line is not necessary
    EndIf

Return

//GOOD ✅
Static Function validateClient()
    Local lClientFound := .F.

    If findClient()
        lClientFound := .T.
    EndIf

Return

Note that in this example, the lClientFound flag will only be true if the findClient() function effectively finds the customer. Therefore, we can initialize it as false and only if the client is found will its content be modified.

Comparative

In AdvPL we have two types of comparatives: “=” (equal) and “==” (exactly equal). I do not recommend using the first one because in some cases of string comparison we can have a false positive (partially true).

In the example above, we saw that when comparing two values, but with different operators, our final result was also different? This is the case of a false positive return. Therefore, always choose to use strictly equal (==) to avoid causing major problems in your code and lost hours of sleep.

Bonus

Below are some alternatives to bring more elegance to your code:

To	Prefer
xFilial	FWxFilial
isInCallStack	FWIsInCallStack
cFilEmp	FWCodEmp
cFilAnt	FWCodFil

Well guys, these were some tips that I use in my developments, and as previously stated, this is not a convention, but rather a way of raising the quality of your applications.

That's all folks 👋🏻

References

https://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882

https://www.totvs.com/blog/developers/advpl/