Como manejo a discordância em uma revisão de código em relação a um caso de borda improvável?

an obscure case that is extremely unlikely to happen--in fact I'm not sure it is even possible to occur

Não ter comportamentos não testados no código pode ser muito importante. Se um pedaço de código é executado, por ex. 50 vezes por segundo, uma chance em um milhão acontecerá aproximadamente a cada 5,5 horas de tempo de execução. (No seu caso, as chances parecem menores.)

Você pode falar sobre as prioridades com seu gerente (ou com qualquer pessoa mais experiente responsável pela unidade em que você trabalha). Você entenderá melhor se, por exemplo, Trabalhar no desempenho do código ou código à prova de balas é a principal prioridade, e quão improvável esse caso pode ser. Seu revisor também pode ter uma ideia distorcida de prioridades. Tendo conversado com a pessoa responsável, você terá um tempo mais fácil (des) concordando com as sugestões do seu revisor, e terá algo para se referir.

É sempre uma boa ideia ter mais de um revisor. Se o seu código for revisado apenas por um colega, peça a alguém que conheça esse código, ou a base de código em geral, para dar uma olhada. Uma segunda opinião, novamente, ajudará você a mais facilmente (des) concordar com as sugestões do revisor.

Ter vários comentários recorrentes durante várias revisões de código geralmente indica que uma coisa maior não está sendo comunicada com clareza e os mesmos problemas aparecem repetidas vezes. Tente descobrir essa coisa maior e discuta-a diretamente com o revisor. Faça perguntas por que . Isso me ajudou muito quando comecei a prática de revisões de código.

Eu posso dar um exemplo de um caso que nunca poderia ocorrer e que causou um desastre.

Quando o Ariane 4 estava sendo desenvolvido, os valores da lateral accelerometers foram dimensionados para caber em um número inteiro assinado de 16 bits e porque a saída máxima possível dos acelerômetros, quando escalonados, poderia nunca exceder exceder 32767 e o mínimo poderia nunca cair abaixo de -32768 houve "não há necessidade de a sobrecarga de verificação de intervalo". Em geral, todas as entradas devem ser verificadas antes de qualquer conversão, mas, neste caso, isso estaria tentando capturar um caso impossível.

Vários anos depois, o Ariane 5 estava sendo desenvolvido e o código para escalonar os acelerômetros laterais foi reutilizado com testes mínimos foi "comprovado em uso". Infelizmente, o foguete maior poderia esperar acelerações laterais maiores para que os acelerômetros fossem atualizados e pudessem produzir maiores 64 bits valores de float .

Esses valores maiores "empacotados" no código de conversão, lembram nenhuma verificação de intervalo, e os resultados no primeiro lançamento em 1996 não foram bons. Custou milhões à empresa e causou um grande hiato no programa.

Opontoqueestoutentandofazeréquevocênãodeveignorarcasosdetestecomonuncaacontecendo,extremamenteimprovável,etc.:ospadrõesparaosquaiselesestavamcodificandochamadoparaverificaçãodeintervalodeallvaloresdeentradaexternos.Seissotivessesidotestadoemanipulado,odesastrepoderiatersidoevitado.

NotaquenoAriane4issonãoeraumbug,(comotudofuncionoubemparatodososvalorespossíveis)-foiumafalhaemseguirospadrões.Quandoocódigofoireutilizadoemumcenáriodiferente,elefalhoucatastroficamente,enquanto,seointervalodevalorestivessesidocortado,provavelmenteteriafalhadonormalmenteeaexistênciadeumcasodetesteparaessepoderiadesencadearamumarevisãodosvalores.Éimportantenotartambémque,enquantooscodificadoresetestadoresvieramparaalgumascríticasdosinvestigadoresapósaexplosão,agestão,QA&liderançaficoucomamaioriadaculpa.

Esclarecimento

Emboranemtodosoftwaresejadesegurançacrítica,nemtãoespetacularquandoelefalha,minhaintençãoeradestacarquetestes"impossíveis" ainda podem ter valor. Este é o caso mais dramático que conheço, mas a robótica também pode produzir alguns resultados desastrosos.

Como não foi tratado antes, está fora do escopo do seu esforço. Você ou seu colega podem perguntar ao seu gerente se vale a pena o esforço para cobrir este caso.

Com algoritmos complexos, é muito difícil provar que você pensou em todos os casos de teste que aparecerão no mundo real. Quando você deixa intencionalmente um caso de teste quebrado porque ele não aparece no mundo real, você está potencialmente deixando outros casos de teste quebrados que você nem imaginou ainda.

O outro efeito que acontece com frequência é quando você lida com casos de teste adicionais, seu algoritmo necessariamente se torna mais geral e, por causa disso, você encontra maneiras de simplificá-lo e torná-lo mais robusto para todo seu teste casos. Isso economiza seu tempo em manutenção e solução de problemas no futuro.

Além disso, há muitos casos de "isso nunca deve acontecer" erros acontecendo na natureza. Isso porque seu algoritmo pode não mudar, mas suas entradas podem mudar anos a fio quando ninguém se lembra desse caso de uso não manipulado. É por isso que os desenvolvedores experientes lidam com esse tipo de coisa quando estão atualizados. Ele volta para te morder mais tarde, se você não fizer isso.

Esta não é uma questão técnica, mas uma decisão de estratégia de negócios. Você percebe que a correção sugerida é para um caso muito obscuro, o qual quase nunca acontece. Aqui, faz uma grande diferença se você estiver programando um brinquedo ou se estiver programando um equipamento médico ou um drone armado. As consequências de um mau funcionamento raro serão muito diferentes.

Ao fazer revisões de código, você deve aplicar um entendimento básico das prioridades de negócios ao decidir quanto investir no tratamento de casos raros. Se você não concordar com seu colega em sua interpretação das prioridades do negócio, convém discutir com alguém do lado comercial.

Revisões de código não são puramente sobre correção de código. Na realidade, isso está bem abaixo na lista de benefícios, por trás do compartilhamento de conhecimento e de ser um processo lento, mas constante, para criar um consenso de estilo / design da equipe.

Como você está experimentando, o que conta como "correto" é muitas vezes discutível, e todo mundo tem seu próprio ângulo sobre o que é isso. Na minha experiência, tempo e atenção limitados também podem tornar as revisões de código altamente inconsistentes - o mesmo problema pode ser detectado ou não dependendo de diferentes desenvolvedores e em momentos diferentes pelo mesmo desenvolvedor. Revisores na mentalidade de "o que melhoraria este código?" frequentemente sugerirão mudanças que eles não acrescentariam aos seus próprios esforços naturalmente.

Como codificador experiente (mais de 15 anos como desenvolvedor), muitas vezes sou revisado por codificadores com menos anos de experiência do que eu. Às vezes, eles me perguntam por mudanças que eu discordo levemente, ou acho que não é importante. No entanto, ainda faço essas alterações. Luto batalhas por mudanças apenas quando sinto que o resultado final causaria uma fraqueza no produto, em que o custo do tempo é muito alto, ou onde um ponto de vista "correto" pode ser feito de forma objetiva (por exemplo, a mudança sendo solicitada) t trabalhar na linguagem que estamos usando, ou um benchmark mostra que uma melhoria de desempenho alegada não é uma).

Então sugiro escolher suas batalhas com cuidado. Dois dias codificando um caso de teste que você acha que não é necessário provavelmente não vale o tempo / esforço para lutar. Se você estiver usando uma ferramenta de revisão, como solicitações de pull do GitHub, então talvez comente sobre custo / benefício que você perceba, a fim de anotar sua objeção, mas concorde em continuar a fazer o trabalho. Isso conta como um retrocesso suave, de modo que o revisor saiba que está atingindo um limite e, mais importante, inclua sua justificativa para que casos como esse possam ser escalados se entrarem em conflito. Você quer evitar a escalada de desacordos escritos - você não quer ter um argumento de estilo de fórum da Internet sobre diferenças de trabalho - então pode ser útil falar primeiro sobre o assunto e registrar um resumo justo do resultado da discussão, mesmo se você ainda concordam em discordar sobre a necessidade do trabalho (é inteiramente possível que uma breve discussão amigável decida por ambos).

Após o evento, este é um bom tópico para revisão de sprints ou reuniões de planejamento da equipe de desenvolvimento, etc. Apresente-o da forma mais neutra possível. "Na revisão de código, o Desenvolvedor A identificou este caso de teste adicional, levou dois dias extras para escrever, a equipe acha que a cobertura extra foi justificada a esse custo?" - esta abordagem funciona muito melhor se você realmente fizer o trabalho, como mostra uma luz positiva; você fez o trabalho e apenas deseja pesquisar a equipe em busca de aversão ao risco versus desenvolvimento de recursos.

Eu aconselho você a, pelo menos, afirmar contra o caso obscuro. Dessa forma, os futuros desenvolvedores não apenas verão que você decidiu ativamente contra o caso, mas com um bom manuseio de falhas, que já deveria estar em vigor, isso também surpreenderia.

E, em seguida, faça um caso de teste que declare essa falha. Desta forma, o comportamento é melhor documentado e aparecerá em testes unitários.

Esta resposta, obviamente, assume que o seu julgamento do mesmo ser "extremamente improvável, se possível" está correto e não podemos julgar isso. Mas se for, e seu colega concordar, então uma afirmação explícita contra o evento deve ser uma solução satisfatória para ambos.

Desde que você parece ser novo lá, há apenas uma coisa que você pode fazer - verifique com o líder da equipe (ou líder do projeto). 13 horas é uma decisão de negócios; para algumas empresas / equipes, muito; para alguns, nada. Não é sua decisão, ainda não.

Se o líder disser "cobrir esse caso", tudo bem; se ele diz "nah, estrague tudo", tudo bem - a decisão dele, a responsabilidade dele.

Quanto às revisões de código em geral, relaxe. Ter uma tarefa devolvida a você uma ou duas vezes é perfeitamente normal.

Uma coisa que eu acho que não vi foi abordada em espécie, embora tenha sido criada na resposta do @SteveBarnes:

Quais são as repercussões de um fracasso?

Em alguns campos, uma falha é um erro em uma página da web. Um PC azul telas e reinicia.

Em outros campos, é a vida ou a morte - o carro que dirige sozinho trava. O pacemaker médico deixa de funcionar. Ou na resposta de Steve: as coisas explodem, resultando na perda de milhões de dólares.

Existe um mundo de diferença nesses extremos.

Se 13 horas para cobrir uma "falha" vale a pena, em última análise, não cabe a você. Deve ser da gerência e dos proprietários. Eles devem ter uma idéia do quadro maior.

Você deve ser capaz de dar uma boa ideia do que valerá a pena. Seu robô simplesmente diminuirá a velocidade ou parará? Desempenho degradado? Ou será que uma falha do robô causará danos monetários? Perda de vida?

A resposta a essa pergunta deve direcionar a resposta para "vale 13 horas do horário das empresas". Aviso: Eu disse o horário das empresas. Eles pagam as contas e, finalmente, decidem o que vale a pena. Sua gerência deve dar a palavra final de qualquer forma.

Talvez conversar com a pessoa responsável por priorizar o trabalho? Na inicialização, pode ser o CTO ou o proprietário do produto. Ele poderia ajudar a descobrir se esse trabalho extra é necessário e por quê. Além disso, você pode trazer suas preocupações durante os levantamentos diários (se você os tiver).

Se não houver responsabilidade clara (por exemplo, proprietário do produto) para planejar o trabalho, tente conversar com as pessoas ao seu redor. Mais tarde, pode se tornar um problema que todos estão levando o produto para a direção oposta.

A melhor maneira de lidar com a discordância é a mesma, independentemente de você ser um desenvolvedor júnior ou um desenvolvedor sênior, ou até mesmo um CEO.

Aja como o Columbo .

Se você nunca viu nenhum Columbo, foi um show fantástico. Columbo era esse personagem muito despretensioso - a maioria das pessoas pensava que ele era um pouco louco e não vale a pena se preocupar. Mas parecendo humilde, e apenas pedindo às pessoas que explicassem, ele foi capaz de conseguir o seu homem.

Acho que também está relacionado ao método socrático .

Em geral, você quer fazer perguntas a você mesmo e aos outros para se certificar de que está fazendo as escolhas certas. Não de uma posição de "estou certo, você está errado", mas de uma posição de descoberta honesta. Ou pelo menos da melhor maneira possível.

No seu caso, você tem duas ideias aqui, mas elas têm basicamente o mesmo objetivo: melhorar o código.

Você tem a impressão de que poupar na cobertura de código para um caso potencialmente improvável (impossível?) em favor do desenvolvimento de outros recursos é a melhor maneira de fazer isso.

Seu colega de trabalho tem a impressão de que ter mais cuidado com os casos de canto é mais valioso.

O que eles veem que você não vê? O que você vê que eles não veem? Como um desenvolvedor júnior, você está realmente em uma ótima posição, porque você naturalmente deve fazer perguntas. Em outra resposta, alguém menciona quão surpreendentemente provável é um caso específico. Então você pode começar com: "Ajude-me a entender - fiquei com a impressão de que X, Y e Z - o que está faltando? Por que o framboozle widget? Eu estava sob a impressão de que seria fintável em circunstâncias cromulent. Will o bastão swizzle realmente embiggen os pincéis ANZA? "

Ao questionar suas suposições e suas descobertas, você descobrirá quais são os vieses e, eventualmente, descobrirá qual é o curso de ação correto.

Comece com a suposição de que todos em sua equipe são perfeitamente racionais e eles têm os melhores interesses da equipe e do produto em mente, assim como você. Se eles estão fazendo algo que não faz sentido, então você precisa descobrir o que você não sabe que eles fazem, ou o que você sabe que eles não sabem.

13 horas não é tão importante, eu apenas faria isso. Lembre-se que você está sendo pago por isso. Apenas giz-lo como "segurança no emprego". Também é melhor manter um bom carma entre a equipe. Agora, se fosse algo que levaria uma semana ou mais, você poderia envolver seu gerente e perguntar se esse é o melhor uso do seu tempo, especialmente se você não concordar com isso.

No entanto, parece que você precisa de influência no seu grupo. Veja como você consegue alavancagem: peça perdão, não peça permissão. Acrescente coisas ao programa como achar melhor (dentro do escopo do curso, ou seja, certifique-se de que resolve completamente o problema que o chefe deseja), e diga ao gerente ou seus colegas depois do fato. Não pergunte a eles: "Tudo bem se eu adicionar o recurso X". Em vez disso, basta adicionar recursos que você deseja pessoalmente no programa. Se eles ficarem chateados com um novo recurso ou não concordarem, não há problema em removê-lo. Se eles gostarem, guarde.

Além disso, sempre que lhe pedirem para fazer alguma coisa, vá a "milha extra" e adicione muitas coisas que eles esqueceram de mencionar ou coisas que funcionariam melhor do que o que eles disseram. Mas não pergunte se é "ok" ir a milha extra. Basta fazê-lo e casualmente dizer-lhes sobre isso depois de feito. O que você está fazendo é treiná-los ..

O que acontecerá será que seu gerente o colocará como um "empreendedor" e começará a depositar sua confiança em você, e seus colegas começarão a vê-lo como o líder que você está começando a ser o proprietário do programa. E então, quando coisas como o que você menciona acontecerem no futuro, você terá mais a dizer, porque você é essencialmente a estrela da equipe e os companheiros de equipe recuarão se você não concordar com eles.

A revisão de código atende a vários propósitos. Um que você está obviamente ciente é, " Este código é adequado para um propósito? " Em outras palavras, é funcionalmente correto; é adequadamente testado; são as partes não óbvias devidamente comentadas; está de acordo com as convenções do projeto?

Outra parte da revisão de código é o compartilhamento de conhecimento sobre o sistema. É uma oportunidade para o autor e o revisor aprenderem sobre o código alterado e como ele interage com o resto do sistema.

Um terceiro aspecto é que ele pode fornecer uma revisão dos problemas que existiam antes de quaisquer alterações serem feitas. Muitas vezes, ao revisar as alterações de outra pessoa, localizo algo que perdi em uma iteração anterior (muitas vezes algo de minha autoria). Uma declaração como "Aqui está uma oportunidade para tornar isso mais robusto do que era", não é uma crítica, e não a tome como uma só!

Minha equipe atual considera a revisão de código não apenas como um gateway ou obstáculo que o código deve passar incólume antes de confirmar, mas principalmente como uma oportunidade para uma discussão estruturada de uma área específica de funcionalidade. É uma das ocasiões mais produtivas para o compartilhamento de informações. (E essa é uma boa razão para compartilhar a revisão em torno da equipe, em vez de sempre o mesmo revisor).

Se você perceber as revisões de código como uma atividade de confronto, essa é uma profecia auto-realizável. Se, em vez disso, você considerá-los como a parte mais colaborativa de seu trabalho, eles estimularão melhorias contínuas em seu produto e em como você trabalha em conjunto. Ajuda se uma revisão puder ser clara sobre as prioridades relativas de suas sugestões - há uma diferença entre "Eu gostaria de um comentário útil aqui" e "Isso quebra se x for negativo", por exemplo.

Tendo feito um monte de declarações gerais acima, como isso se aplica à sua situação específica? Espero que agora seja óbvio que meu conselho é responder à revisão com perguntas abertas e negociar qual abordagem é mais valiosa. Para o seu caso de exemplo em que um teste extra é sugerido, algo como: "Sim, podemos testar isso; eu estimo que ele levará tempo para ser implementado. Você acha que o benefício vale E existe outra coisa que podemos fazer para garantir que o teste não seja necessário? "

Uma coisa me impressiona quando leio sua pergunta: se é necessário um esforço de dois dias para escrever um novo caso de teste, então seu novo teste é um cenário muito diferente dos seus testes existentes (caso em que provavelmente ele muito valor) ou você identificou a necessidade de reutilização de código aprimorada no conjunto de testes.

Finalmente, como um comentário geral sobre o valor das revisões de código (e como um resumo do que eu disse acima), eu gosto desta declaração, em Maintainers não são dimensionados por Daniel Vetter :

At least for me, review isn’t just about ensuring good code quality, but also about diffusing knowledge and improving understanding. At first there’s maybe one person, the author (and that’s not a given), understanding the code. After good review there should be at least two people who fully understand it, including corner cases.

O código pode SEMPRE ser melhor.

Se você está em uma revisão de código e não vê nada que possa ser melhor ou um teste de unidade que possa detectar um bug, não é o código perfeito, mas o revisor que não está fazendo o trabalho deles. Se você escolheu mencionar a melhoria é uma escolha pessoal. Mas quase sempre que sua equipe faz uma revisão de código, deve haver coisas que alguém perceba que poderiam ser melhores ou que todos provavelmente perderiam seu tempo.

Dito isto, se você age com base nos comentários ou não, depende da sua equipe. Se suas alterações corrigirem o problema ou adicionarem valor suficiente sem alteração que sua equipe as aceita, junte-as e registre seus comentários no backlog para que alguém o discuta posteriormente. Se a equipe descobrir que suas alterações adicionam mais risco ou complexidade do que o valor, será necessário resolver os problemas de acordo.

Lembre-se de que qualquer código tem pelo menos mais um caso extremo que poderia ser testado e poderia usar pelo menos mais uma refatoração. É por isso que as revisões de código são feitas melhor como um grupo, com todos olhando para o mesmo código ao mesmo tempo. Para que no final todos possam chegar a um consenso sobre se o código em revisão é aceitável (como é) e agrega valor suficiente para se fundir à base da comunidade ou se certas coisas devem ser feitas antes que haja valor suficiente para mesclar .

Como você está fazendo esta pergunta, presumo que você não está realmente fazendo "revisões de código", mas sim criando uma solicitação pull ou outro mecanismo de envio para que outros possam comentar de maneira não determinística. Então agora você está em um problema de gerenciamento e uma definição de feito. Eu acho que o seu gerenciamento é indeciso e não entende o processo e propósito das revisões de código e provavelmente não tem uma "definição de feito" (DOD). Porque se eles fizessem o seu DOD explicitamente responderia a esta pergunta e você não teria que vir aqui e perguntou.

Como você conserta isso? Bem, peça a seu gerente que lhe dê um DOD e diga se você precisa sempre implementar todos os comentários. Se a pessoa em questão é seu gerente, então a resposta é evidente.

Esta questão não é sobre as virtudes da programação defensiva, os perigos de casos específicos ou os riscos catastróficos de erros em produtos físicos. Na verdade, nem é mesmo sobre engenharia de software .

Há duas coisas que você precisa apreciar sobre ser um desenvolvedor júnior. Em primeiro lugar, significa que, embora seja possível que você esteja certo, e ele esteja errado, é - no balanço das probabilidades - improvável. Se o seu colega de trabalho está fazendo uma sugestão de que você não pode ver o valor de, você precisa considerar seriamente a possibilidade de não ter experiência suficiente para compreendê-lo. Eu não entendo esse sentido deste post.

A segunda coisa a apreciar é que o seu sócio sênior é chamado porque ele tem mais responsabilidade. Se um júnior quebra alguma coisa importante, eles não ficarão em apuros se seguirem as instruções. Se um senior