Cobertura de código destaca métodos não utilizados - o que devo fazer?

59

Fui encarregado de aumentar a cobertura de código de um projeto Java existente.

Notei que a ferramenta de cobertura de código ( EclEmma ) destacou alguns métodos que nunca são chamados de qualquer lugar.

Minha reação inicial é não para escrever testes de unidade para esses métodos, mas para destacá-los ao meu gerente de linha / equipe e perguntar por que essas funções estão lá para começar.

Qual seria a melhor abordagem? Escreva testes unitários para eles ou pergunte por que eles estão lá?

    
por Lucas T 28.05.2018 / 10:52
fonte

7 respostas

119
  1. Excluir.
  2. Commit.
  3. Esqueça.

Fundamentação:

  1. O código morto está morto. Por sua própria descrição, não tem propósito. Pode ter tido um propósito em um ponto, mas isso se foi, e então o código deve desaparecer.
  2. O controle de versão garante que, no (na minha experiência) evento único raro que alguém aparece mais tarde procurando esse código, ele pode ser recuperado.
  3. Como um efeito colateral, você melhora instantaneamente a cobertura de código sem fazer nada (a menos que o código morto seja testado, o que raramente é o caso).

Advertência dos comentários: A resposta original pressupunha que você tinha verificado completamente que o código está, sem dúvida, morto. Os IDEs são falíveis, e há várias maneiras em que o código que parece morto pode, de fato, ser chamado. Traga um especialista, a menos que você esteja absolutamente certo.

    
por 28.05.2018 / 10:58
fonte
55

Todas as outras respostas são baseadas no pressuposto de que os métodos em questão são realmente não utilizados. No entanto, a pergunta não especificou se esse projeto é autônomo ou uma biblioteca de algum tipo.

Se o projeto em questão é uma biblioteca, os métodos aparentemente não usados podem ser usados fora do projeto e removê-los quebraria esses outros projetos. Se a própria biblioteca for vendida a clientes ou disponibilizada publicamente, pode ser até mesmo impossível rastrear o uso desses métodos.

Neste caso, existem quatro possibilidades:

  • Se os métodos forem privados ou privados de pacotes, eles poderão ser removidos com segurança.
  • Se os métodos forem públicos, sua presença poderá ser justificada mesmo sem o uso real, para a integridade dos recursos. Eles devem ser testados embora.
  • Se os métodos forem públicos e desnecessários, removê-los será uma alteração urgente e se a biblioteca seguir a versão semântica , isso só será permitido em uma nova versão principal.
  • Como alternativa, os métodos públicos também podem ser reprovados e removidos posteriormente. Isso dá algum tempo para que os consumidores de API façam a transição das funções reprovadas antes de serem removidas na próxima versão principal.
por 28.05.2018 / 18:02
fonte
31

Primeiro verifique se sua ferramenta de cobertura de código está correta.

Eu tive situações em que eles não pegaram em métodos sendo chamados através de referências à interface, ou se a classe é carregada dinamicamente em algum lugar.

    
por 28.05.2018 / 12:04
fonte
15

Como o Java é estaticamente compilado, deve ser bastante seguro remover os métodos. Remover código morto é sempre bom. Existe alguma probabilidade de que exista algum sistema de reflexão maluca que os execute em tempo de execução, portanto, verifique primeiro com outros desenvolvedores, mas remova-os.

    
por 28.05.2018 / 10:59
fonte
9

What would the best approach be? Write unit tests for them, or question why they're there?

Excluir código é bom.

Quando você não pode excluir o código, certamente pode marcá-lo como @Deprecated , documentando qual importante release que você está direcionando para remover o método. Então você pode excluí-lo "mais tarde". Nesse meio tempo, ficará claro que nenhum código novo deve ser adicionado que dependa dele.

Eu não recomendaria investir em métodos obsoletos - portanto, nenhuma nova unidade testa apenas para atingir as metas de cobertura.

A diferença entre os dois é principalmente se os métodos são ou não parte da interface publicada . A exclusão arbitrária de partes da interface publicada pode ser uma surpresa desagradável para os consumidores que dependiam da interface.

Eu não posso falar com EclEmma, mas pelas minhas experiências uma das coisas que você precisa ter cuidado é a reflexão. Se, por exemplo, você usar arquivos de configuração de texto para escolher quais classes / métodos acessar, a distinção usada / não usada pode não ser óbvia (eu já queimei isso com um tempo acoplado).

Se o seu projeto é uma folha no gráfico de dependência, o caso de depreciação é enfraquecido. Se o seu projeto é uma biblioteca, o caso da reprovação é mais strong.

Se sua empresa usa um reporte mono , a exclusão é um risco menor do que no caso de vários repo.

Como observado por l0b0 , se os métodos já estiverem disponíveis no controle de origem, recuperá-los após a exclusão é um procedimento direto exercício. Se você estava realmente preocupado com a necessidade de fazer isso, pense em como organizar seus commits para poder recuperar as alterações excluídas, se precisar delas.

Se a incerteza for alta o suficiente, você pode considerar comentar o código , em vez de excluí-lo. É um trabalho extra no caminho feliz (em que o código excluído nunca é restaurado), mas facilita a restauração. Meu palpite é que você deve preferir uma exclusão direta até que você tenha sido gravada por um par de vezes, o que lhe dará algumas idéias sobre como avaliar a "incerteza" neste contexto.

question why they're there?

O tempo investido na captura do conhecimento não é necessariamente desperdiçado. Eu sou conhecido por realizar uma remoção em duas etapas - primeiro, adicionando e confirmando um comentário explicando o que aprendemos sobre o código e depois excluindo o código (e o comentário).

Você também pode usar algo análogo a registros de decisões arquiteturais como forma de capturar o lore com o código fonte.

    
por 28.05.2018 / 16:27
fonte
9

Uma ferramenta de cobertura de código não é onisciente, tudo vê. Só porque sua ferramenta afirma que o método não é chamado, isso não significa que ele não é chamado. Há reflexão e, dependendo da linguagem, pode haver outras maneiras de chamar o método. Em C ou C ++, pode haver macros que construam nomes de funções ou métodos, e a ferramenta pode não ver a chamada. Portanto, o primeiro passo seria: fazer uma pesquisa textual do nome do método e dos nomes relacionados. Pergunte a colegas experientes. Você pode achar que é realmente usado.

Se você não tiver certeza, coloque uma afirmação () no início de cada método "não utilizado". Talvez seja chamado. Ou uma declaração de log.

Talvez o código seja realmente valioso. Pode ser um novo código em que um colega está trabalhando há duas semanas e que ele ou ela ligaria amanhã. Não é chamado hoje porque o chamado será adicionado amanhã.

Talvez o código seja realmente valioso, parte 2: O código pode estar realizando alguns testes de tempo de execução muito caros que poderiam descobrir que as coisas estão dando errado. O código só é ativado se as coisas realmente derem errado. Você pode estar excluindo uma ferramenta valiosa de depuração.

Curiosamente, o pior conselho possível é "Excluir. Confirmar. Esqueça." é a melhor classificação. (Revisões de código? Você não faz revisões de código? O que você está fazendo programando se você não faz revisões de código?)

    
por 28.05.2018 / 20:33
fonte
6

Dependendo do ambiente em que o software é executado, você pode registrar se o método já foi chamado. Se não for chamado dentro de um período de tempo adequado, o método pode ser removido com segurança.

Esta é uma abordagem mais cautelosa do que apenas excluir o método e pode ser útil se você estiver executando em um ambiente altamente sensível a falhas.

Registramos em um canal de #unreachable-code slack dedicado com um identificador exclusivo para cada candidato para remoção, e ele provou ser bastante eficaz.

    
por 28.05.2018 / 19:26
fonte