advpl-refactoring — Padrões de refactor com before/after

advpl-refactoring — Padrões de refactor com before/after

Esses 6 padrões cobrem ~80% das oportunidades de refactor em fontes ADVPL legados (Protheus R10/R11) ou customizações novas escritas sem o conhecimento dos idiomas modernos. Cada um tem gatilho de detecção (como achar via plugadvpl), before/after lado-a-lado e quando NÃO refatorar (custo pode superar valor).

Workflow

1. Antes de propor refactor: rode /plugadvpl:arch <arquivo> pra entender escopo, /plugadvpl:callers <funcao> pra impacto downstream, e /plugadvpl:lint <arquivo> pra confirmar que há findings.
2. 🚨 Refactor envolve Edit em massa — .prw cp1252 precisa stage/commit OBRIGATÓRIO:

   plugadvpl edit-prw stage <arquivo.prw>   # cp1252 -> utf-8 (cria .bak)
   # ... agora aplique os refactors via Edit/Write ...
   plugadvpl edit-prw commit <arquivo.prw>  # utf-8 -> cp1252 (restaura)
   

   Sem isso, acentos nos comentários/strings não-editados viram ? e o compilador AppServer rejeita o arquivo. Bug silencioso. Detalhes: /plugadvpl:edit-prw.
3. Critical/error do lint geralmente é refactor candidato — comece pelos worst offenders.
4. Refactor um padrão por commit. Misturar 3 refactors num diff só gera review impossível e bug oculto.
5. Depois do refactor: /plugadvpl:reindex <arquivo> + /plugadvpl:lint <arquivo> pra confirmar que não introduziu regressão (findings que não existiam antes).
6. Antes de commitar git: /plugadvpl:edit-prw check <arquivo> confirma que voltou pra cp1252 (match: true).

---

1. DbSeek em loop → SQL embarcado (anti-N+1)

Gatilho — /plugadvpl:lint --regra PERF-002. Ou simplesmente: vê DbSeek dentro de For/While/iteração em alias com muitas linhas, isso é N round-trips no banco.

Por que refatorar: N consultas DbSeek viram N idas ao DBAccess. Reescrever como JOIN em SQL embarcado faz 1 ida só. Em produção típica (1.000 itens), reduz tempo de minutos para segundos.

Before

DbSelectArea("SC6")
DbSetOrder(1)
DbSeek(xFilial("SC6") + cPedido)
While !Eof() .And. SC6->C6_NUM == cPedido
    // Pra cada item, busca preço/saldo na SB1
    DbSelectArea("SB1")
    DbSetOrder(1)
    If DbSeek(xFilial("SB1") + SC6->C6_PRODUTO)
        nPreco := SB1->B1_PRV1
        cDesc  := SB1->B1_DESC
    EndIf
    DbSelectArea("SC6")
    SC6->(DbSkip())
EndDo

After

BeginSql Alias "QRY"
    SELECT SC6.C6_PRODUTO, SC6.C6_QTDVEN,
           SB1.B1_PRV1, SB1.B1_DESC
      FROM %table:SC6% SC6
      LEFT JOIN %table:SB1% SB1
             ON SB1.B1_FILIAL  = %xfilial:SB1%
            AND SB1.B1_COD     = SC6.C6_PRODUTO
            AND SB1.%notDel%
     WHERE SC6.C6_FILIAL = %xfilial:SC6%
       AND SC6.C6_NUM    = %exp:cPedido%
       AND SC6.%notDel%
EndSql
TCSetField("QRY", "B1_PRV1", "N", 12, 4)

While !QRY->(Eof())
    nPreco := QRY->B1_PRV1
    cDesc  := QRY->B1_DESC
    QRY->(DbSkip())
EndDo
QRY->(DbCloseArea())

Quando NÃO refatorar:

• Loop tem menos de 10 iterações garantidas — overhead de escrever SQL não compensa.
• Lógica entre DbSeeks é muito complexa (15+ ramificações) — fica ilegível em SQL.

---

2. Posicione() repetido → cache em variável

Gatilho — /plugadvpl:grep "Posicione\(". Procure múltiplos Posicione() na MESMA função buscando o MESMO alias/chave.

Por que refatorar: Posicione() é uma função-wrapper que faz SaveArea + DbSelectArea

• DbSetOrder + DbSeek + RestArea por baixo. Cada chamada custa ~5-10ms. 3 chamadas iguais = 30ms desperdiçados. Pior em loop.

Before

cNomeCli := Posicione("SA1", 1, xFilial("SA1") + cCodigo, "A1_NOME")
nLimite  := Posicione("SA1", 1, xFilial("SA1") + cCodigo, "A1_LC")
cTipo    := Posicione("SA1", 1, xFilial("SA1") + cCodigo, "A1_TIPO")

After

// Faz 1 posicionamento, lê todos os campos da MEMÓRIA do alias.
cNomeCli := ""
nLimite  := 0
cTipo    := ""
DbSelectArea("SA1")
DbSetOrder(1)
If MsSeek(xFilial("SA1") + cCodigo)
    cNomeCli := SA1->A1_NOME
    nLimite  := SA1->A1_LC
    cTipo    := SA1->A1_TIPO
EndIf

Variante com FwFwmGetValueByID / FWFormStruct (MVC): se você está em ModelDef, prefira oModel:GetValue(...) em vez de Posicione.

Quando NÃO refatorar:

• Você precisa APENAS UM campo e está em código que não é hotpath.
• Lógica intermediária pode invalidar o alias (outra função muda WorkArea no meio).

---

3. Lista de If hardcoded → tabela SX5/SX6 ou User Function central

Gatilho — /plugadvpl:grep "^\s*If\s.*==" em arquivo com 10+ matches; ou ramificações do tipo If cCod == 'A' .Or. cCod == 'B' .Or. ... com listas de literais.

Por que refatorar: Hardcoded é doloroso pra mudar (precisa de compilação + deploy). Em Protheus a infraestrutura SX5 (tabela genérica) e SX6 (parâmetros MV_*) existem exatamente pra isso.

Before

// 47 IFs hardcoded pra descobrir grupo do produto
Static Function _GrupoProduto(cCod)
    Local cGrupo := ""
    If Left(cCod, 2) == "01"
        cGrupo := "ELETRO"
    ElseIf Left(cCod, 2) == "02"
        cGrupo := "MOVEIS"
    ElseIf Left(cCod, 2) == "03"
        cGrupo := "ROUPAS"
    // ... mais 44 ramificações
    EndIf
Return cGrupo

After — opção A: SX5 (tabela genérica)

// Cadastra na SX5 (Configurador) tabela "ZG" = grupos por prefixo
// X5_CHAVE = '01' → X5_DESCRI = 'ELETRO', etc.
Static Function _GrupoProduto(cCod)
    Local cPrefixo := Left(cCod, 2)
    Local cGrupo   := ""
    DbSelectArea("SX5")
    DbSetOrder(1)
    If MsSeek(xFilial("SX5") + "ZG" + cPrefixo)
        cGrupo := AllTrim(SX5->X5_DESCRI)
    EndIf
Return cGrupo

After — opção B: User Function central + DEFINE

// Em ZUTILS.prw — função compartilhada, mudança em 1 lugar
User Function GrpProd(cCod)
    Local cPrefixo := Left(cCod, 2)
    Local cGrupo   := ""
    Do Case
        Case cPrefixo == "01"  ; cGrupo := "ELETRO"
        Case cPrefixo == "02"  ; cGrupo := "MOVEIS"
        // ...
    EndCase
Return cGrupo

Quando NÃO refatorar:

• Lista tem 5 entradas ou menos que nunca mudam — hardcoded é OK.
• Performance crítica em hotpath de loop massivo — Do Case é mais rápido que DbSeek em SX5.

---

4. AxCadastro / Modelo2 / Modelo3 → MVC (MenuDef/ModelDef/ViewDef)

Gatilho — /plugadvpl:lint --regra MOD-004.

Por que refatorar: AxCadastro e Modelo2/3 são wrappers procedurais legados que não plugam no framework MVC moderno (eventos, validações cruzadas, FWMVCRotAuto pra testes automatizados, multi-camada Tela/Modelo/View). Customizações novas em R12+ devem ser MVC.

Before

User Function ZA1CAD()
    Local cAlias := "ZA1"
    Local aRotina := { ...; }
    DbSelectArea(cAlias)
    DbSetOrder(1)
    AxCadastro(cAlias, "Cadastro Customizado", "AllwaysTrue()", "AllwaysTrue()")
Return

After (esqueleto MVC — três Static Functions)

#Include 'Protheus.ch'
#Include 'FWMVCDef.ch'

User Function ZA1MVC()
    Local oBrw := FWMBrowse():New()
    oBrw:SetAlias("ZA1")
    oBrw:SetDescription("Cadastro Customizado")
    oBrw:Activate()
Return

Static Function MenuDef()
    Local aRotina := {}
    aAdd(aRotina, {"Pesquisar"  , "PesqBrw"        , 0, 1})
    aAdd(aRotina, {"Visualizar" , "FWVIEWMODELO('VIEWDEF.ZA1MVC')", 0, 2})
    aAdd(aRotina, {"Incluir"    , "FWEXECVIEW('Inclusao', 'VIEWDEF.ZA1MVC', 3)", 0, 3})
    aAdd(aRotina, {"Alterar"    , "FWEXECVIEW('Alteracao', 'VIEWDEF.ZA1MVC', 4)", 0, 4})
    aAdd(aRotina, {"Excluir"    , "FWEXECVIEW('Exclusao', 'VIEWDEF.ZA1MVC', 5)", 0, 5})
Return aRotina

Static Function ModelDef()
    Local oStru := FWFormStruct(1, "ZA1")
    Local oModel := MPFormModel():New("ZA1MVC", /*bPre*/, /*bPos*/, /*bCommit*/)
    oModel:AddFields("ZA1MASTER", /*cOwner*/, oStru)
    oModel:SetDescription("Cadastro Customizado ZA1")
Return oModel

Static Function ViewDef()
    Local oModel := FWLoadModel("ZA1MVC")
    Local oStru  := FWFormStruct(2, "ZA1")
    Local oView  := FWFormView():New()
    oView:SetModel(oModel)
    oView:AddField("ZA1MASTER_VIEW", oStru, "ZA1MASTER")
    oView:CreateHorizontalBox("MAIN", 100)
    oView:SetOwnerView("ZA1MASTER_VIEW", "MAIN")
Return oView

Veja [[advpl-mvc-avancado]] pra padrões avançados (eventos, FWMVCRotAuto).

Quando NÃO refatorar:

• Cadastro já está em produção há anos sem queixa e ninguém pediu mudança — refactor cego pode introduzir bug numa rotina estável. Espere o próximo bug/feature pra fazer junto.

---

5. Concatenação de string em loop → array + Array2String

Gatilho — /plugadvpl:lint --regra PERF-004. Ou: += ou + aplicado a string dentro de While/For com 100+ iterações.

Por que refatorar: Em ADVPL, string é imutável. Cada cVar += "x" aloca string nova, copia conteúdo antigo + "x", e descarta o anterior. 1.000 concatenações = 1.000 alocações, 500.000 chars copiados. Acumular em array e fazer 1 join no final é O(n) em vez de O(n²).

Before

Local cCsv := ""
DbSelectArea("SA1")
DbGoTop()
While !Eof()
    cCsv += AllTrim(SA1->A1_COD) + ";"
    cCsv += AllTrim(SA1->A1_NOME) + ";"
    cCsv += DtoC(SA1->A1_DTCAD) + Chr(13) + Chr(10)
    SA1->(DbSkip())
EndDo

After

Local aLinhas := {}
DbSelectArea("SA1")
DbGoTop()
While !Eof()
    aAdd(aLinhas, AllTrim(SA1->A1_COD)  + ";" + ;
                  AllTrim(SA1->A1_NOME) + ";" + ;
                  DtoC(SA1->A1_DTCAD))
    SA1->(DbSkip())
EndDo
cCsv := FwArrayJoin(aLinhas, Chr(13) + Chr(10))
// (em versões anteriores ao FwArrayJoin existir, usar aJoin ou loop com Replace)

Quando NÃO refatorar:

• Loop com < 50 iterações — diferença é imperceptível.
• Você precisa do estado parcial da string a cada iteração — array não te dá isso barato.

---

6. RecLock solto → Begin Transaction block

Gatilho — /plugadvpl:lint --regra BP-001 e BP-005. RecLocks em múltiplos aliases sem proteção transacional.

Por que refatorar: Sem Begin Transaction/End Transaction, se a primeira gravação commitar e a segunda falhar, fica estado inconsistente (ex.: pedido sem item). Begin Transaction garante atomicidade — se algo dá erro no meio, faz rollback automático.

Before

RecLock("SC5", .T.)
SC5->C5_FILIAL := xFilial("SC5")
SC5->C5_NUM    := cNumPed
SC5->C5_CLIENTE := cCliente
SC5->(MsUnlock())   // já commitou

// Se ZErrEnter() der erro aqui, SC5 ficou orfao
ZErrEnter()

RecLock("SC6", .T.)
SC6->C6_FILIAL := xFilial("SC6")
SC6->C6_NUM    := cNumPed
SC6->C6_ITEM   := "01"
SC6->(MsUnlock())

After

Begin Transaction
    RecLock("SC5", .T.)
    SC5->C5_FILIAL  := xFilial("SC5")
    SC5->C5_NUM     := cNumPed
    SC5->C5_CLIENTE := cCliente
    SC5->(MsUnlock())

    ZErrEnter()   // se der erro aqui, transaction faz rollback de SC5 tambem

    RecLock("SC6", .T.)
    SC6->C6_FILIAL := xFilial("SC6")
    SC6->C6_NUM    := cNumPed
    SC6->C6_ITEM   := "01"
    SC6->(MsUnlock())
End Transaction

Combinado com Begin Sequence/Recover/End para captura de erro:

Begin Sequence
    Begin Transaction
        // ... gravações
    End Transaction
Recover Using oErr
    ConOut("Erro: " + oErr:Description)
    DisarmTransaction()  // garante rollback explicito
End Sequence

Quando NÃO refatorar:

• Apenas 1 alias sendo gravado e o erro é tratado em VALID — RecLock+MsUnlock simples basta.
• Função roda dentro de transaction maior (chamadora já abriu) — Begin Transaction aninhado tem semântica diferente em alguns DBs; verifique.

---

Workflow de proposta de refactor

Quando o usuário pede "melhorar" / "está lento" / "tem boa prática aqui?":

1. /plugadvpl:arch <arquivo> — entende a função, tabelas, deps.
2. /plugadvpl:lint <arquivo> --severity warning — colhe candidatos.
3. Para cada finding, identifique qual dos 6 padrões aplica.
4. Mostre o diff curto (não reescreva o arquivo inteiro) com antes/depois.
5. Liste caveats: testes a rodar, impacto em callers, deps.
6. Após aplicar, /plugadvpl:reindex + /plugadvpl:lint pra confirmar.

Anti-padrões de refactor

• Refactor sem benchmark / sem caso de teste: "achei que ia ser melhor" não é refactor, é mudança gratuita.
• Refactor com 5 padrões num diff só: review impossível, bug oculto certo.
• Modernizar AxCadastro em rotina estável sem demanda: se está em produção e funciona, espere o próximo trigger (bug, nova feature) pra fazer junto.
• Substituir Posicione por DbSeek sem testar eof: Posicione protege contra alias inválido; DbSeek bruto não.

Comandos plugadvpl relacionados

• /plugadvpl:lint <arquivo> --severity warning — colhe candidatos de refactor.
• /plugadvpl:arch <arquivo> — contexto antes de propor mudanças.
• /plugadvpl:callers <funcao> — impacto downstream de mudar a assinatura.
• /plugadvpl:grep "Posicione\(" — busca uso de funções a refatorar.
• [[advpl-code-review]] — as 24 regras que disparam refactor candidate.
• [[advpl-mvc]] / [[advpl-mvc-avancado]] — padrão MVC pra modernização.
• [[advpl-embedded-sql]] — SQL embarcado pra refactor de DbSeek loops.