Painel de líderes

Conheça a loja dos produtos do Projeto ACBr, e ajude o projeto a crescer, com estilo. https://loja.projetoacbr.com.br Poucos sabem, mas dia 31 de janeiro é o dia do boné! Seja pela estética, ou pela proteção contra o sol, os bonés estão por todos os lados. Nossos bonés personalizados contam com qualidade em todos os processos de sua fabricação, como mostra o Grupo Kyoodai neste vídeo abaixo. Confira como são feitos os bonés do Projeto ACBr: Conheça a loja dos produtos do Projeto ACBr, onde você pode adquirir uma dessas obras-primas, e muitos outros produtos do projeto. Ajude o projeto a crescer, com estilo. https://loja.projetoacbr.com.br

Foto por David Siglin em Unsplash. Olá pessoal, É bom quando encontramos uma ferramenta que facilita ou melhora nosso trabalho, não? Todos devem ter notado que ultimamente temos enviado vários commits ao SVN de remoção de warnings e hints, muitas vezes mencionando a ferramenta FixInsight. Para quem não conhece, essa ferramenta faz uma análise do seu código e aponta possíveis erros e sugere otimizações. Ela é uma ferramenta muito boa, tanto que foi comprada pela TMS e se tornou TMS FixInsight. Já tem um tempo que conheço a ferramenta e sempre tive o desejo de rodá-la em todo o código do ACBr. Mas devido ao tempo não tinha sido possível. Depois de um incentivo (valeu @Waldir Paim), eu resolvi baixar a versão trial e fazer isso. E que bom que fiz. Gostaríamos de compartilhar com vocês algumas coisas que encontramos no nosso código com a ajuda dessa ferramenta. Encontrando pequenos problemas num código gigante Vamos começar por um código que estava no ACBrValidador. Vejam esse código, onde a função ValidarCEP de baixo chama a função ValidarCEP de cima, e tente encontrar um problema: function ValidarCEP(const ACEP, AUF: String): String; begin Result := ValidarDocumento( docCEP, ACEP, AUF); end; function ValidarCEP(const ACEP: Integer; AUF: String): String; begin ValidarCEP( FormatarCEP(ACEP), AUF ); end; Conseguiu ver o problema? Essa função nunca retornaria que um CEP é inválido se você passasse o CEP como inteiro. Precisava de um “Result := ” no início. Simples? Nem tanto quando lembramos do tamanho do projeto ACBr. Temos mais de 200 componentes e mais de 779 mil linhas de código, contribuídos por dezenas ou talvez centenas de programadores, embora a nossa equipe de commiters seja realmente pequena. Só a unit ACBrValidador.pas em questão tem atualmente cerca de 2070 linhas. Não fica muito mais fácil quando uma ferramenta aponta pra você? [FixInsight Warning] ACBrValidador.pas(294): W521 Return value of function 'ValidarCEP' might be undefined Vamos a outro exemplo no pacote ACBrSerial, componente ACBrECF: [FixInsight Warning] ACBrECFDaruma.pas(4638): W503 Assignment right hand side is equal to its left hand side Veja o código (só a parte interessante): else if StrToIntDef(fsNumVersao, -1) >= 345 then begin RetCmd := EnviaComando( ESC + #240 ); RetCmd := Copy(RetCmd, 92, Length(RetCmd)); RetCmd := RetCmd; //<--- Viu aqui??? for A := 0 to fpAliquotas.Count-1 do begin fpAliquotas[A].Total := RoundTo( StrToFloatDef(Copy(RetCmd,(A*14)+1,14),0) / 100, -2 ); end; end; end; Uma linha que não faz absolutamente nada a não ser gastar espaço, memória e CPU. Uma linha desnecessária a menos no código. E você consegue encontrar um no seu aplicativo código que nunca será executado? Ainda no mesmo pacote, veja esse exemplo:  [FixInsight Warning] ACBrECFDataRegis.pas(1838): W509 Unreachable code Nesse código: if (fsArqPrgBcoTXT <> '') and (not FileExists( fsArqPrgBcoTXT )) then begin Msg := ACBrStr( 'Arquivo '+fsArqPrgBcoTXT+' não encontrado. '+ 'Valores padrões serão utilizados.' ) ; raise EACBrECFErro.Create( Msg ); fsArqPrgBcoTXT := '' ; //Essa linha nunca vai ser executada porque tem um raise acima. end ; Mais uma vez, tente imaginar procurar esse problema num projeto tão grande. Não é facilmente percebido se você não tiver olhos treinados e estiver procurando problemas. Vamos a outro exemplo ainda no componente ACBrECF:  [FixInsight Warning] ACBrECFEscECF.pas(1222): W517 Variable 'CHK' hides a class field, method or property Veja esse código: procedure TACBrECFEscECFResposta.SetResposta(const AValue: AnsiString); Var Soma, I, F, LenCmd : Integer ; CHK : Byte ; begin O problema desse código é que ele confunde uma variável local (CHK) com uma propriedade da classe (TACBrECFEscECFResposta.CHK). É preciso analisar todo código em cada lugar que isso acontece para ter certeza quando você está se referindo a propriedade e quando é a variável. Imagine se você confunde uma com a outra. Uma hora você pensa que sua variável está recebendo valores estranhos. Outra hora você pensa que sua propriedade não está sendo atualizada. Nesse caso específico, a variável foi renomeada para vCHK evitando a confusão com a propriedade CHK. O importante é que quando você for ler o código, não precise ficar pensando “Isso aqui é uma variável ou uma propriedade?”. Veja outro exemplo, agora no ACBrSMS: [FixInsight Warning] ACBrSMSClass.pas(192): W511 Object 'ListaSMS' created in TRY block begin try Self.Clear; if not FileExists(APath) then raise EACBrSMSException.CreateFmt('Arquivo "%s" não encontrado.', [APath]); ListaSMS := TStringList.Create; ListaSMS.LoadFromFile(APath); if ListaSMS.Count = 0 then Exit; //(bla bla bla...) finally FreeAndNil(ListaSMS); end; Não é apropriado esse código. O correto é mover a criação do objeto para fora do try..finally. Pense bem, se o objeto não for construído, você não quer que ele seja destruído. A mensagem ajudou a perceber também que esse bloco poderia ser escrito de outra maneira. Aquele raise não precisava estar dentro do try..finally. Evitando problemas futuros Rodando no pacote ACBrOpenSSL tivemos a seguinte mensagem no componente ACBrEAD: [FixInsight Optimization] ACBrEAD.pas(268): O804 Method parameter 'AChavePublicaOpenSSL' is declared but never used Quer dizer, parâmetro ‘AchavePublicaOpenSSL’ declarado mas não utilizado. Veja abaixo a a parte importante da função: function TACBrEAD.ConverteChavePublicaParaOpenSSH( const AChavePublicaOpenSSL: String): String; Var Buffer, Modulo, Expoente: AnsiString; {...} begin // https://www.netmeister.org/blog/ssh2pkcs8.html CalcularModuloeExpoente(Modulo, Expoente); Buffer := EncodeBufferSSH('ssh-rsa') + EncodeHexaSSH(Expoente) + EncodeHexaSSH('00'+Modulo); Result := 'ssh-rsa '+ EncodeBase64(Buffer); end; É estranho esse método ConverteChavePublicaParaOpenSSH não utilizar o parâmetro da chavePública. Qualquer pessoa que visse o método e tentasse chamar passando a chave pública não teria o resultado desejado. Analisando o código melhor vemos que o componente lê a chave pública por meio do método “LerChavePublica”. Nesse caso o correto seria remover o parâmetro para que não haja nenhuma confusão. E essa mensagem no TACBrBALToledo2090: [FixInsight Warning] ACBrBALToledo2090.pas(107): W508 Variable is assigned twice successively if (Length(wStrListDados[1]) = 16) then wDecimais := 1000; {APENAS BLOCO PROCESSADO} wResposta := wStrListDados[1]; //<---- sobreposto pela linha seguinte wResposta := Copy(wStrListDados[1], 5, 7); if (Length(wResposta) <= 0) then Exit; Veja que os dados de uma linha é sobreposta pela outra. O compilador nunca daria um aviso sobre isso. Mais dois exemplos de mensagens e o código a seguir: [FixInsight Warning] ACBrEscEpsonP2.pas(97): W514 Loop iterator could be out of range (missing -1?) [FixInsight Warning] ACBrEscEpsonP2.pas(100): W514 Loop iterator could be out of range (missing -1?) For I := 0 to Length(cTAGS_BARRAS) do TagsNaoSuportadas.Add( cTAGS_BARRAS[I] ); For I := 0 to Length(cTAGS_ALINHAMENTO) do TagsNaoSuportadas.Add( cTAGS_ALINHAMENTO[I] ); Essa eu não sei como não foi detectada antes. Por algum motivo não está sendo emitida a mensagem estouro quando o valor de I chega a 16 no primeiro caso e 3 no segundo. Encontrando erros gerados por Ctrl+C..Ctrl+V No pacote ACBrPAF veja a mensagem gerada: [FixInsight Optimization] ACBrPAF_T_Class.pas(137): O804 Method parameter 'ACampo2' is declared but never used function OrdenarT2(const ACampo1, ACampo2: Pointer): Integer; var Campo1, Campo2: String; begin Campo1 := FormatDateTime('YYYYMMDD', TRegistroT2(ACampo1).DT_MOV) + TRegistroT2(ACampo1).TP_DOCTO + TRegistroT2(ACampo1).SERIE + TRegistroT2(ACampo1).NUM_ECF; Campo2 := FormatDateTime('YYYYMMDD', TRegistroT2(ACampo1).DT_MOV) + TRegistroT2(ACampo1).TP_DOCTO + TRegistroT2(ACampo1).SERIE + TRegistroT2(ACampo1).NUM_ECF; Result := AnsiCompareText(Campo1, Campo2); end; Essa função é utilizada para ordenar os registros T2 do PAF. Mas veja que ela compara o registro “ACampo1” com ele mesmo. Suspeita: Ctrl+C e Ctrl+V... Quem nunca??... Outra situação diferente, mas relacionada com ordenação apareceu no ACBrSintegra. Na verdade 4 situações no ACBrSintegra, semelhantes entre si. Vou mostrar apenas uma, mas dessa vez a mensagem do FixInsight fica pra depois. Vamos a um jogo dos sete erros entre os ifs e else no código abaixo: function Sort60A(Item1, Item2: Pointer): Integer; var witem1, witem2 : TRegistro60A; begin witem1 := TRegistro60A(Item1); witem2 := TRegistro60A(Item2); if witem1.Emissao>witem2.Emissao then begin if witem1.NumSerie>witem2.NumSerie then Result:=1 else if witem1.NumSerie=witem2.NumSerie then Result:=0 else Result:=-1; end else if witem1.Emissao = witem2.Emissao then begin if witem1.NumSerie>witem2.NumSerie then Result:=1 else if witem1.NumSerie=witem2.NumSerie then Result:=0 else Result:=-1; end else begin if witem1.NumSerie>witem2.NumSerie then Result:=1 else if witem1.NumSerie=witem2.NumSerie then Result:=0 else Result:=-1; end; end; Conseguiu encontrar os erros? Bem, se você procurou diferenças, não deve ter encontrado nada. E não existe mesmo. Veja a mensagem da ferramenta: [FixInsight Warning] ACBrSintegra.pas(3410): W507 THEN statement is equal to ELSE statement São dois if e um else pra fazer a mesma coisa... A correção foi remover o IFs e ELSE.  Agora vamos ao pacote ACBrSPED. Depois de remover muitos e muitos parâmetros desnecessários apontados pelo FixInsight, veja esse código: function CodAjToStr(const AValue: TACBrCodAj): string; begin if AValue = codAjAcaoJudicial then Result := '01' else if AValue = codAjAcaoJudicial then Result := '02' else if AValue = codAjLegTributaria then Result := '03' else if AValue = codAjEspRTI then Result := '04' else if AValue = codAjOutrasSituacaoes then Result := '05' else if AValue = codAjEstorno then Result := '06'; end; A mensagem é a seguinte: [FixInsight Warning] ACBrEPCBlocos.pas(2071): W512 Odd ELSE-IF condition (review lines 2071 and 2073) Viu lá? Os dois primeiros ifs estão comparando AValue com o mesmo valor, "codAjAcaoJudicial". O segundo deveria ser "codAjProAdministrativo". Provavelmente mais um Ctrl+C..Ctrl+V. Mensagens para otimização de código Nem todas as mensagens geradas são de erros. Algumas são mensagens de otimização. Muitos dos commits que temos feito estão relacionados a uma mensagem como estas abaixo: [FixInsight Optimization] ACBrSATClass.pas(776): O801 CONST missing for unmodified string parameter 'CNPJvalue' [FixInsight Optimization] ACBrSATClass.pas(776): O801 CONST missing for unmodified string parameter 'assinaturaCNPJs' Ela pode ser gerada numa função como essa: function TACBrSATClass.AssociarAssinatura( CNPJvalue, assinaturaCNPJs : AnsiString) : String ; begin ...// um código que não altera nenhum dos parâmetros citados end; Essas mensagens estão dizendo que os parâmetros 'CNPJvalue' e ‘assinaturaCNPJs’ do tipo string não estão sendo alterados dentro da função a que eles pertencem. Nesse caso é bem provável que os parâmetros devessem ter um prefixo CONST na sua declaração, como abaixo: function TACBrSATClass.AssociarAssinatura(const CNPJvalue, assinaturaCNPJs : AnsiString) : String ; begin ...// um código que não altera nenhum dos parâmetros citados end; Não vou entrar em muitos detalhes sobre isso, mas usar CONST tem alguns benefícios, principalmente em caso de strings: A execução é mais rápida, porque o compilador pode otimizar o código. No caso de strings, não tem contagem de referências; O compilador garante que você não vai alterar os parâmetros passados gerando um efeito colateral indesejado em quem chamou as funções; O código fica mais legível, porque você pode ler que a intenção é não alterar o parâmetro passado; Como os parâmetros são imutáveis, pode tornar o código mais ThreadSafe; Se quer saber um pouco mais sobre isso, recomendo os seguintes links: All hail the “const” parameters! Is the use of ‘const’ dogmatic or rational? Concluindo... Bom pessoal, ainda temos bastante pra fazer. Contudo, queremos dizer que o FixInsight tem nos ajudado melhorar nosso código. Ficamos tão satisfeitos que entramos em contato com a TMS e eles generosamente nos cederam uma licença da versão Pro pra continuar nosso trabalho. Muito obrigado TMS. Agora, se você quer nossa opinião, essa é uma ferramenta altamente recomendada e está disponível pra toda versão do Delphi a partir do Delphi 2006. Se você tem alguma dúvida, baixe a versão trial e comece agora mesmo a usar no seu código. A versão trial limita as mensagens a 5 por units e funciona por 30 dias. Mas é o suficiente pra se perceber como é muito útil, como aconteceu com a gente. Quer um passo a passo em como utilizá-la? Veja o próximo post logo abaixo.