Alguém pode desafiar o tio Bob em seu amor de remover "aparelhos inúteis"?

93

Eu odeio referenciar conteúdo paywalled, mas este vídeo mostra exatamente do que estou falando. Precisamente 12 minutos em Robert Martin olha para isso:

Ediz:"Uma das minhas coisas favoritas para fazer é livrar-se de chaves inúteis", como ele se transforma nisso:

Hámuitotempoatrás,emumaeducaçãomuitodistante,eufuiensinadoanãofazerissoporquefacilitaaintroduçãodeumbugadicionandooutralinharecuadapensandoqueelaécontroladapeloifquandonãoé.

ParaserjustocomotioBob,eleestárefatorandoumlongométodoJavaempequenasfunçõesque,euconcordo,sãomuitomaislegíveis.Quandoeleacabademudarisso,(22.18),éassim:

Euestouquerendosaberseissodevevalidararemoçãodaschaves.Jáestoufamiliarizadocomas melhores práticas . O tio Bob pode ser desafiado neste ponto? O tio Bob defendeu a idéia?

    
por candied_orange 04.06.2016 / 01:03
fonte

10 respostas

46

A legibilidade não é pequena.

Eu tenho uma mentalidade mista quando se trata de chaves que envolvem um único método. Eu, pessoalmente, removê-los para coisas como declarações de retorno de linha única, mas deixar essas chaves de fato nos incomodou muito no último lugar onde trabalhei. Alguém adicionou uma linha de código a uma instrução if sem adicionar as chaves necessárias e, como era C, ela travou o sistema sem avisar.

Eu nunca desafio alguém que é religioso a sempre usar chaves depois desse pequeno fiasco.

Então, vejo o benefício da legibilidade, mas estou ciente dos problemas que podem surgir quando você deixa esses aparelhos fora.

Eu não me incomodaria em tentar encontrar um estudo ou a opinião de alguém. Todo mundo tem um (uma opinião) e, por ser uma questão estilística, uma opinião é tão boa quanto qualquer outra. Pense no problema sozinho, avalie os prós e contras e crie sua própria mente maldita. Se a loja em que você trabalha tem um padrão de codificação que cobre esse problema, basta seguir isso.

    
por 04.06.2016 / 02:07
fonte
132

Você pode encontrar várias promoções ou rejeições publicadas de estilos sem chave em aqui ou aqui ou onde quer que galpões de bicicletas sejam pintados.

Afastando-se dos galpões de bicicleta, lembre-se do ótimo bug SSL do OS X / iOS de 2014?

if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
    goto fail;
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    goto fail;
    goto fail;
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
    goto fail;

Sim, "causado" por blocos sem colchetes link

As preferências podem depender do estilo de chave. Se eu tivesse que escrever

if (suiteSetup)
{
    includePage(mode, suiteSetup);
}

Eu posso estar inclinado a economizar espaço também. Mas

if (suiteSetup) {
    includePage(mode, suiteSetup);
}

usa apenas uma linha "extra".

Eu sei que você não perguntou, mas se estou trabalhando sozinha, sou herege. Se eu remover chaves, prefiro

if (suiteSetup != null) includePage(mode, suiteSetup);

Não tem o mesmo problema visual que o bug do estilo SSL do iOS, mas economiza ainda mais linhas "desnecessárias" do que o Uncle Bob;) Lê bem se você está acostumado com isso e tem uso predominante em Ruby (%código%). Ah, bem, apenas saiba que há muitas opiniões.

    
por 04.06.2016 / 06:27
fonte
61

O tio Bob tem muitas camadas de defesa contra esse erro que não eram tão comuns quando "sempre usa chaves" era a sabedoria predominante:

  1. Uma strong preferência pessoal por condicionais de linha única, portanto, os de várias linhas se destacam e recebem um exame extra.
  2. Um editor que automaticamente se desdobra quando você insere uma segunda linha.
  3. Um conjunto completo de testes de unidade que ele executa constantemente.
  4. Um conjunto completo de testes de integração.
  5. Uma revisão feita antes que seu código seja mesclado.
  6. Uma nova execução de todos os testes por um servidor de integração contínua.
  7. Teste feito por um departamento de qualidade do produto.

Acho que se alguém publicasse um desafio para o tio Bob, ele teria uma boa resposta com os pontos acima. Este é um dos erros mais fáceis de pegar cedo.

    
por 04.06.2016 / 02:57
fonte
31

Para a maior parte, esta é uma preferência pessoal, no entanto, há algumas coisas a considerar.

Possíveis bugs

Embora possa ser argumentado que os erros causados pelo esquecimento das chaves de add-in são raros, pelo que visto que eles fazer acontecer ocasionalmente (sem esquecer o famoso IOS goto falha bug). Então eu acho que isso deve ser um fator ao considerar seu estilo de código (algumas ferramentas alertam sobre indentação enganosa , por isso depende da sua cadeia de ferramentas também) .

Código válido (que diz que pode ser um bug)

Mesmo supondo que seu projeto não sofra tais erros, ao ler o código você pode ver alguns blocos de código que parecem poderiam ser bugs - mas não são, tirando um pouco da sua mentalidade ciclos.

Começamos com:

if (foo)
    bar();

Um desenvolvedor adiciona um comentário útil.

if (foo)
    // At this point we know foo is valid.
    bar();

Mais tarde, um desenvolvedor se expande.

if (foo)
    // At this point we know foo is valid.
    // This never fails but is too slow even for debug, so keep disabled.
    // assert(is_valid(foo));
    bar();

Ou adiciona um bloco aninhado:

if (foo)
    while (i--) {
        bar(i);
        baz(i);
    }

Ou usa uma macro:

if (foo)
    SOME_MACRO();

"... Como as macros podem definir várias linhas de código, a macro usa do {...} while (0) para várias linhas? Deve porque está no nosso guia de estilo, mas é melhor verificar apenas no caso!"

Os exemplos acima são todos códigos válidos, no entanto, quanto mais conteúdo no bloco de códigos, mais você precisa ler para garantir que não haja erros.

Talvez seu estilo de código defina que blocos de várias linhas exigem uma chave (não importa o que, mesmo que não seja código) , mas eu vi esses tipos de comentários serem adicionados código de produção. Quando você lê, há uma pequena dúvida de que quem quer que tenha editado as linhas por último esqueceu-se de adicionar uma chave, às vezes eu sinto que a necessidade de verificar novamente está funcionando como pretendido (especialmente ao investigar um bug nessa área do código ) .

Diff Noise

Um motivo prático para usar chaves para linhas simples é reduzir o diff noise .

Ou seja, mudando:

if (foo)
    bar();

Para:

if (foo) {
    bar();
    baz();
}

... faz com que a linha condicional apareça em um diff como sendo alterado, isso adiciona um pequeno mas desnecessário overhead.

  • as linhas aparecem como alteradas em revisões de código, se suas ferramentas de diferenciação forem baseadas em palavras, você pode ver facilmente que apenas a chave foi alterada, mas isso leva mais tempo para verificar se a linha não foi alterada Dito isto, nem todas as ferramentas suportam o diff baseado em palavras, o diff (svn, git, hg ... etc) mostrará como se toda a linha mudasse, mesmo com ferramentas sofisticadas, às vezes você pode precisar procurar rapidamente sobre um diff baseado em linha simples para ver o que mudou.
  • As ferramentas de anotação
  • (como git blame ) mostrarão a linha como alterada, tornando o rastreamento da origem de uma linha mais uma etapa para encontrar a alteração real .

Estes são pequenos e dependem de quanto tempo você gasta em revisão de código ou rastreamento que confirma linhas de código alteradas.

Uma inconveniência mais tangível de ter linhas extras alteradas em um diff, sua maior probabilidade de alteração no código causará conflitos que precisam ser resolvidos manualmente .

Existe uma exceção a isso, para bases de código que possuem { em sua própria linha - não é um problema.

O argumento diff noise não é válido se você escrever neste estilo:

if (foo)
{
    bar();
    baz();
}

No entanto, isso não é uma convenção tão comum, então, principalmente, adicionando à resposta a completude (não sugerindo que os projetos usem esse estilo) .

    
por 04.06.2016 / 08:11
fonte
15

Anos atrás, fui trazido para depurar código C. O bug era muito difícil de encontrar, mas acabou se resumindo a uma declaração como:

if (debug)
   foo (4);

E descobriu-se que a pessoa que o havia escrito definiu foo como macro. Uma macro com duas linhas de código. E, claro, apenas a primeira dessas duas linhas estava sujeita ao if . (Então a segunda linha foi executada incondicionalmente.)

Isto pode ser absolutamente único para C e seu pré-processador - que faz substituições antes da compilação - mas eu nunca esqueci isso. Esse tipo de coisa deixa uma marca em você, e por que não jogar pelo seguro - especialmente se você usa uma variedade de linguagens e toolchains e não pode ter certeza de que essas travessuras não são possíveis em outros lugares?

Agora eu recuo e uso chaves de maneira diferente de todos os outros, aparentemente. Para uma única linha if , eu faria:

if (debug) { foo (4); }

para que não sejam necessárias linhas adicionais para incluir as chaves.

    
por 04.06.2016 / 15:49
fonte
14

"Tio Bob" pode ter sua opinião, você pode ter sua opinião. Não há necessidade de desafiá-lo.

Se você quer um apelo à autoridade, leve Chris Lattner. No Swift, se as instruções perdem seus parênteses, mas sempre vêm com chaves. Nenhuma discussão, é parte da linguagem. Então, se "Uncle Bob" começar a remover chaves, o código para de compilar.

Passar pelo código de outra pessoa e "livrar-se de aparelhos inúteis" é um mau hábito. Apenas causa trabalho extra quando o código precisa ser revisado e quando conflitos são desnecessariamente criados. Talvez "Uncle Bob" seja um programador tão incrivelmente bom que ele não precise de resenhas de código? Eu desperdicei uma semana da minha vida porque um dos principais programadores mudou "if (p! = NULL)" para "if (! P)" sem uma revisão de código, escondida no pior lugar possível.

Este é principalmente um debate de estilo inofensivo. Chaves têm a vantagem de poder inserir outra linha de código sem adicionar chaves. Como uma declaração de log, ou um comentário (se seguido por comentário seguido por declaração é simplesmente horrível). declaração na mesma linha como se tem a desvantagem prática que você tem problemas com muitos depuradores. Mas faça o que você preferir.

    
por 05.06.2016 / 09:43
fonte
8

Minhas razões para não remover chaves são:

  1. reduz a fadiga de decisão. Se você sempre usa chaves, você nunca precisa decidir se vai precisar de chaves ou não.

  2. reduza o arrasto de desenvolvimento: mesmo se você tentar eventualmente extrair todas as múltiplas linhas de lógica para métodos, ter que converter um braceless se for um bracing se adicionar lógica for um pouco irritante arrastar desenvolvimento. Portanto, há o esforço de remover as chaves e o esforço de adicioná-las novamente quando você precisar de mais código. Minúscula, mas irritante.

por 06.06.2016 / 08:08
fonte
0

Um jovem colega de trabalho disse que os aparelhos que consideramos redundantes e supérfluos são de fato úteis para ele. Não me lembro exatamente por quê, mas se isso permite que ele escreva um código melhor mais rápido, só isso é motivo para mantê-los.

Fwiw, nós concordamos em um compromisso que colocá-los em uma linha não faz pré-condições tão curtas un para me agradar / distrair. Por exemplo,

if (!param) { throw invalid_argument (blahblah); }
if (n < 2) { return 0; }
// real code for function starts here...

Também indico que essas pré-condições introdutórias são frequentemente declarações de fluxo de controle para o corpo (por exemplo, return ), então o medo de adicionar uma segunda instrução que está sob a mesma condição e esquecer de escrever chaves é discutível . Uma segunda declaração sob a condição seria código morto de qualquer maneira e não faz sentido.

Suspeito que o problema da fluência na leitura é causado pelo parser cerebral da pessoa que está sendo conectado para ter as chaves como parte da declaração condicional. Isso não é uma representação precisa da gramática de C ++, mas poderia ser um efeito colateral de aprender certas outras línguas primeiro, onde esse é o caso.

    
por 05.06.2016 / 21:19
fonte
-1

Tendo assistido a esse vídeo agora, percebi que a eliminação de chaves facilita a visualização de onde o código ainda precisa ser refatorado. Se você precisar de chaves, seu código provavelmente ficará um pouco mais limpo.

Tendo dito isso, quando você está em um time, a eliminação de aparelhos provavelmente levará a um comportamento inesperado algum dia. Eu digo mantê-los, e você vai economizar muitas dores de cabeça quando as coisas dão errado.

    
por 05.10.2016 / 11:47
fonte
-3

I was taught not to do this because it makes it easy to introduce a bug by adding another indented line thinking it's controlled by the if when it's not.

Se seu código for bem testado em unidade , esse tipo de "bug" explodirá na cara.

Limpar o código evitando os colchetes inúteis e feios é uma prática que eu sigo.

    
por 06.06.2016 / 03:22
fonte