Manutenção de código: Para adicionar comentários no código ou apenas deixá-lo no controle de versão?

42

Fomos solicitados a adicionar comentários com tags iniciais, tags finais, descrição, solução, etc. para cada alteração que fizermos no código como parte da correção de um bug / implementação de um CR.

Minha preocupação é que isso fornece algum valor agregado? Como é, temos todos os detalhes no histórico de controle de versão, o que nos ajudará a acompanhar cada mudança?

Mas meus líderes estão insistindo em ter os comentários como uma prática de programação "boa". Um de seus argumentos é quando um CR precisa ser desescovado / alterado, seria complicado se os comentários não estivessem lá.

Considerando que as mudanças estariam amplamente entre os códigos, seria realmente útil adicionar comentários para cada mudança que fizermos? Não devemos deixá-lo para o controle de versão?

    
por Chillax 09.11.2012 / 19:32
fonte

9 respostas

43

Você está absolutamente certo. As alterações de rastreamento são o trabalho do seu sistema de controle de versão. Toda vez que você fizer um commit você deve escrever uma mensagem de commit explicando o que foi feito, e referenciando seu sistema de rastreamento de bugs se esta for uma correção de bug. Colocando um comentário no código dizendo

// begin fix for bug XXXXX on 10/9/2012
...
// end fix for bug XXXXX

toda vez que você consertar um bug, seu código ficará ilegível e inelegível. Também resultará na duplicação da mesma informação em dois lugares, o que tornará a bagunça ainda pior.

Os comentários não devem ser usados para o acompanhamento de bugs e eles também não devem descrever o que seu código está fazendo. Eles devem explicar por que você está fazendo X, ou porque você está fazendo X desta maneira particular. Se você sentir a necessidade de escrever um comentário explicando o que um bloco de código está fazendo, isso é um cheiro de código que indica que você deve refatorar esse bloco em uma função com um nome descritivo.

Então, em vez de

// fixed bug XXXXX on 10/9/2012

você pode ter um comentário que diga

// doing X, because otherwise Y will break.

ou

// doing X, because doing Y is 10 times slower.
    
por 09.11.2012 / 21:39
fonte
53

Use a melhor ferramenta para o trabalho. Seu sistema de controle de versão deve ser a melhor ferramenta para gravar quando correções de erros e CRs são feitos: ele registra automaticamente a data e quem fez a mudança; nunca esquece de adicionar uma mensagem (se você configurou para exigir mensagens de confirmação); nunca anota a linha de código errada ou elimina acidentalmente um comentário. E se o seu sistema de controle de versão já está fazendo um trabalho melhor do que seus comentários, é bobagem duplicar o trabalho adicionando comentários.

A legibilidade do código-fonte é primordial. Uma base de código cheia de comentários que fornecem o histórico completo de todas as correções de erros e CRs não será muito legível.

Mas não pule os comentários completamente: Os comentários (não servindo como documentação para cada start / stop / description / solution de cada bugfix e CR) aumentam a legibilidade do código. Por exemplo, para um código complicado ou impreciso que você adiciona para corrigir um bug, um comentário do formulário // fix ISSUE#413 informando às pessoas onde encontrar mais informações no rastreador de problemas é uma excelente ideia.

    
por 09.11.2012 / 19:50
fonte
7

Os comentários no código são sobre o que o código é naquele momento. Tirar um instantâneo a qualquer momento não deve se referir a versões antigas (ou piores) do código.

Os comentários no VCS são sobre como o código foi alterado. Eles devem ler como uma história sobre desenvolvimento.

Agora, todas as alterações devem incluir comentários? na maioria dos casos, sim. A única exceção que imagino é quando o comportamento esperado já foi documentado, mas não foi o que você conseguiu, devido a um bug. Corrigi-lo torna os comentários existentes mais precisos, para que eles não precisem ser alterados. O bug em si deve ser documentado no histórico do ticket e no comentário de commit, mas somente no código se o código parecer estranho. Nesse caso, um // make sure <bad thing> doesn't happen deve ser suficiente.

    
por 09.11.2012 / 19:46
fonte
6

Um tipo de comentário que eu realmente aprecio é:

// Implementado para a Regra de Negócios 5 da Proposta 2

ou o que diabos você usa para reunir suas necessidades.

Isso tem duas vantagens, uma é que ele permite encontrar o motivo pelo qual você implementou um determinado algoritmo sem pesquisar, e outro é que ele ajudará você a se comunicar com engenheiros que não trabalham com software e que trabalham nos documentos de requisitos.

Isso pode não ajudar com equipes menores, mas se você tiver analistas que desenvolvam suas necessidades, isso pode ser inestimável.

    
por 09.11.2012 / 23:29
fonte
4

Seus leads estão certos quando dizem que os comentários são uma boa prática de programação, mas existem exceções. Adicionar um comentário para cada alteração que você faz é um deles. E você está certo ao dizer que isso deve pertencer ao sistema de controle de versão. Se você tem que manter esses comentários em um único lugar, então o VCS é o caminho a percorrer. Os comentários no código-fonte tendem a ficar antigos e sem manutenção. Nenhum comentário é muito melhor que comentários ruins. O que você não quer é ter comentários em ambos os lugares (no código e no VCS) que estão fora de sincronia. O objetivo é manter as coisas DRY, tendo uma única fonte de verdade para alterações no código.

    
por 09.11.2012 / 19:47
fonte
3

Além do que os outros disseram, considere o que acontece se uma mudança tiver efeitos de ondulação em todo o sistema. Digamos que você refatorie uma parte de uma interface principal no processo de implementação de uma solicitação de mudança - esse tipo de alteração pode facilmente afetar uma grande porcentagem dos arquivos de código-fonte em qualquer software não trivial com alterações triviais (classe ou mudanças no nome do método). Você deveria passar por todos os arquivos tocados por essa operação para anotá-la manualmente com esses comentários, em vez de confiar no VCS fazendo tudo automaticamente? Em um caso, você está olhando para pouco mais que um trabalho de cinco minutos com qualquer ferramenta de refatoração decente, seguida por uma recompilação para garantir que nada tenha quebrado a estrutura, enquanto o outro pode facilmente se transformar em um dia de trabalho. Para que benefício específico?

Considere também o que acontece quando você move partes do código. Um dos desenvolvedores de banco de dados com quem trabalho está no campo da maior parte "cada linha de SQL deve ser anotada com a revisão em que foi alterada, e vamos fazer históricos de revisão separados para cada arquivo, porque é mais fácil ver quem mudou o que quando e porque ". Isso funciona meio que bem quando a mudança é na ordem de mudar as linhas simples. Ele não funciona tão bem quando, como eu fiz recentemente para corrigir um problema sério de desempenho, você divide partes de uma consulta maior introduzindo tabelas temporárias e, em seguida, altera a ordem de algumas das consultas para se adequar melhor ao novo fluxo de código. Concedido, o diff contra a versão anterior foi em grande parte sem sentido, uma vez que disse que cerca de dois terços do arquivo havia mudado, mas o comentário de check-in também foi algo como "reorganização importante para corrigir problemas de desempenho". No momento em que você olhou manualmente para as duas versões, ficou bem claro que as partes grandes realmente eram as mesmas, apenas se moviam. (E levou o procedimento armazenado em questão de regularmente levar mais de meio minuto para executar, para alguns segundos. Naquela época, era em grande parte I / O-bound com poucos lugares para melhorias significativas, pelo menos a curto prazo. )

Com pouquíssimas exceções, alterar o rastreamento e a referência de problemas é o trabalho do VCS, IMNSHO.

    
por 09.11.2012 / 22:36
fonte
3

Eu costumo seguir esta regra: se a mudança é óbvia e o código resultante não levanta questões, não reverte ou altera substancialmente qualquer comportamento anterior de forma substancial - então deixe para o VCS rastrear números de bugs e outras mudanças informação.

No entanto, se houver uma mudança que não seja óbvia, isso altera a lógica - especialmente muda substancialmente a lógica feita por outra pessoa de uma maneira não óbvia - pode ser muito benéfico adicionar algo como "essa mudança é para faça isso e aquilo por causa do bug # 42742 ". Desta forma, quando alguém olha para o código e se pergunta "por que isso está aqui? Isso parece estranho", ele tem uma orientação direta sobre ele e não precisa fazer uma investigação via VCS. Isso também evita situações em que as pessoas quebram as alterações de outras pessoas porque estão familiarizadas com o estado antigo do código, mas não percebem que ele foi alterado desde então.

    
por 10.11.2012 / 08:25
fonte
2

Comentários relacionados ao controle de versão não pertencem ao arquivo de origem. Eles só adicionam desordem. Como é provável que eles precisem ser colocados no mesmo lugar (como um bloco de comentários no topo do arquivo), eles causarão conflitos de interferência somente quando filiais paralelas forem mescladas.

Todas as informações de rastreamento que podem ser retiradas do controle de versão não devem ser duplicadas no corpo do código. Isso vale para ideias tolas como as palavras-chave de checkout do RCS, como $Log$ e sua história.

Se o código já sai do escopo do sistema de controle de versão, essa trilha de comentários sobre seu histórico perde o contexto e, portanto, a maior parte de seu valor. Para compreender corretamente a descrição da alteração, precisamos acessar a revisão, para que possamos visualizar o diff para a versão anterior.

Alguns arquivos antigos no kernel do Linux têm grandes blocos de comentários históricos. Aqueles datam de quando não havia sistema de controle de versão, apenas tarballs e patches.

    
por 09.11.2012 / 23:52
fonte
2

Os comentários no código devem ser mínimos e precisos. Adicionar defeito / alterar informações não valiosas. Você deve usar o controle de versão para isso. Algum tempo O controle de versão fornece uma maneira um pouco melhor de mudança - Usamos o ClearCase UCM; As atividades do UCM são criadas com base nos números de defeitos, na área de alteração, etc. (por exemplo, defect29844_change_sql_to_handle_null).

Comentários detalhados são preferidos nos comentários de check-in.

Prefiro incluir detalhes sobre informações de fundo, detalhes da solução NÃO implementada devido a alguns efeitos colaterais.

Programador Pramagic e CleanCode levam à seguinte diretriz

Mantenha o conhecimento de baixo nível no código, onde ele pertence, e reserve os comentários para outras explicações de alto nível.

    
por 10.11.2012 / 12:53
fonte