Clever Geek Handbook

Por que as revisões de código são boas, mas não o suficiente

image1.png


As revisões de código são definitivamente necessárias e úteis. Esta é uma oportunidade de transferir conhecimento, treinamento, controle sobre a tarefa, melhorar a qualidade e o design do código e corrigir erros. Além disso, você pode notar erros de alto nível associados à arquitetura e aos algoritmos usados. Em geral está tudo bem, mas as pessoas se cansam rapidamente. Portanto, a análise estática é um excelente complemento para revisões e ajuda a identificar uma variedade de erros e erros de impressão que são invisíveis a olho nu. Vejamos um bom exemplo neste tópico.



Tente encontrar o erro no código da função retirado da biblioteca structopt : 



static inline bool is_valid_number(const std::string &input) {
  if (is_binary_notation(input) ||
      is_hex_notation(input) ||
      is_octal_notation(input)) {
    return true;
  }

  if (input.empty()) {
    return false;
  }

  std::size_t i = 0, j = input.length() - 1;

  // Handling whitespaces
  while (i < input.length() && input[i] == ' ')
    i++;
  while (input[j] == ' ')
    j--;

  if (i > j)
    return false;

  // if string is of length 1 and the only
  // character is not a digit
  if (i == j && !(input[i] >= '0' && input[i] <= '9'))
    return false;

  // If the 1st char is not '+', '-', '.' or digit
  if (input[i] != '.' && input[i] != '+' && input[i] != '-' &&
      !(input[i] >= '0' && input[i] <= '9'))
    return false;

  // To check if a '.' or 'e' is found in given
  // string. We use this flag to make sure that
  // either of them appear only once.
  bool dot_or_exp = false;

  for (; i <= j; i++) {
    // If any of the char does not belong to
    // {digit, +, -, ., e}
    if (input[i] != 'e' && input[i] != '.' &&
        input[i] != '+' && input[i] != '-' &&
        !(input[i] >= '0' && input[i] <= '9'))
      return false;

    if (input[i] == '.') {
      // checks if the char 'e' has already
      // occurred before '.' If yes, return false;.
      if (dot_or_exp == true)
        return false;

      // If '.' is the last character.
      if (i + 1 > input.length())
        return false;

      // if '.' is not followed by a digit.
      if (!(input[i + 1] >= '0' && input[i + 1] <= '9'))
        return false;
    }

    else if (input[i] == 'e') {
      // set dot_or_exp = 1 when e is encountered.
      dot_or_exp = true;

      // if there is no digit before 'e'.
      if (!(input[i - 1] >= '0' && input[i - 1] <= '9'))
        return false;

      // If 'e' is the last Character
      if (i + 1 > input.length())
        return false;

      // if e is not followed either by
      // '+', '-' or a digit
      if (input[i + 1] != '+' && input[i + 1] != '-' &&
          (input[i + 1] >= '0' && input[i] <= '9'))
        return false;
    }
  }

  /* If the string skips all above cases, then
  it is numeric*/
  return true;
}


Para não ler acidentalmente a resposta de imediato, adicionarei uma imagem.



image2.png


Não sei se você encontrou um erro ou não. Mesmo se você o tiver encontrado, certamente concordará que não é fácil encontrar esse erro de digitação. Além disso, você sabia que havia um erro na função. Se você não souber, será difícil fazer com que leia com atenção e verifique todo esse código.



Em tais situações, um analisador de código estático complementará perfeitamente a revisão de código clássica. O analisador não se cansa e verificará minuciosamente todo o código. Como resultado, o analisador PVS-Studio percebe uma anomalia nesta função e emite um aviso:



V560 Uma parte da expressão condicional é sempre falsa: input [i] <= '9'. structopt.hpp 1870



Para quem não percebeu o erro, vou dar uma explicação. A coisa mais importante: 



else if (input[i] == 'e') {
  ....
  if (input[i + 1] != '+' && input[i + 1] != '-' &&
      (input[i + 1] >= '0' && input[i] <= '9'))
      return false;
}


A condição acima verifica se o i-ésimo elemento é a letra 'e'. Consequentemente, a seguinte entrada de verificação [i] <= '9' não faz sentido. O resultado da segunda verificação é sempre falso, que é o que a ferramenta de análise estática avisa. O motivo do erro é simples: a pessoa se apressou e se selou, esquecendo de escrever +1.



Na verdade, verifica-se que a função não completa seu trabalho de verificação da exatidão dos números inseridos. Opção correta: 



else if (input[i] == 'e') {
  ....
  if (input[i + 1] != '+' && input[i + 1] != '-' &&
      (input[i + 1] >= '0' && input[i + 1] <= '9'))
      return false;
}


Uma observação interessante. Este erro pode ser visto como uma variação do " efeito da última linha ". O erro foi cometido na última condição da função. No final, a atenção do programador diminuiu e ele cometeu esse erro sutil.





Se você gostar do artigo sobre o efeito da última linha, recomendo a leitura de outras observações semelhantes: 0-1-2 , memset , comparações .



Adeus a todos Gosto de quem descobriu o erro por conta própria.


More articles:


All Articles