Clean Code: Applying in the AdvPL
Tags: cleancode, advpl, totvs, programming
Published: 2024-04-24 14:29:39

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

```go
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.

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

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

Separation of Responsibilities

```go
//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
```

```go
//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!!!!

```go
//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
```

```go
//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 

```go
//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
```

```go
//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.

```go
//BAD ❌
User Function calculateBonus(cEmployer, nSalary)
    Local lClienteEncontrado    := .T.
 
    nSalario += 100
    nSalario *= 2
 
    If nSalary > 1000
        nSalary := nSalary + (0.1 * nSalary)
    EndIf
 
Return
```

```go
//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.

```go
//BAD ❌
Static Function validateClient()
    Local lClientFound := .F.
 
    If findClient()
        lClientFound := .T.
    Else
        lClientFound := .F. //This line is not necessary
    EndIf
 
Return
```

```go
//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).

![Comparative between two strings using only equal and another using two equals](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/mmgt787prnmbciw3pufz.png)

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/