Mau sinal se ninguém consegue compreender o código de alguém? [duplicado]

53

Se um programador escreve código que ninguém além do que ele consegue entender, e as revisões de código sempre terminam com o revisor coçando a cabeça ou segurando a cabeça nas mãos, isso é um sinal claro de que o codificador simplesmente não está desativado? programação profissional? Isso seria suficiente para justificar uma mudança de carreira? Quão importante é o código compreensível neste setor?

Considere estes exemplos C, compare:

if (b)
  return 42;
else
  return 7;

versus:

return (b * 42) | (~(b - 1) * 7);

Existe alguma desculpa para empregar o último exemplo? se sim, quando e porquê?

EDITAR: Deixando o último fragmento original para consideração, adicione uma correção:

return (b * 42) | ((b - 1) & 7);

Eu acho que a outra observação interessante é que requer que b seja 1 para verdadeiro e 0 para falso, quaisquer outros valores produziriam resultados estranhos.

    
por Jonathan Neufeld 28.02.2015 / 01:15
fonte

9 respostas

107

A primeira regra de qualquer engenheiro de software profissional é escrever código que seja compreensível. O segundo exemplo se parece com um exemplo otimizado para um compilador mais antigo, não otimizador, ou apenas alguém que quer se expressar com operadores bit a bit. Está bem claro o que está acontecendo se estivermos familiarizados com operações bit a bit, mas a menos que você esteja em um domínio onde esse é o caso, evite o segundo exemplo. Também devo salientar que as chaves estão faltando no primeiro exemplo.

O codificador pode insistir que seu código é eficiente, pode ser o caso, mas se não puder ser mantido, pode não ser escrito, já que gerará dívida técnica horrenda no futuro.

    
por 28.02.2015 / 01:24
fonte
83

Existem várias perguntas que você levanta.

1) Isso é um sinal claro de que o codificador não é cortado para programação profissional?

  • Não. Os desenvolvedores muitas vezes passam por etapas em que aprendem sobre uma ideia e querem aplicá-la. Eles sempre aplicam essas ideias de maneira eficiente e / ou eficaz. Não. Os erros são cometidos e fazem parte do processo de aprendizagem. Se eles estão consistentemente escrevendo código incompreensível, melhor comunicação é necessária. Os revisores devem comunicar quais são as expectativas para o código aceitável, e o codificador deve comunicar o que o código está (supostamente) fazendo e, se necessário, por que foi feito de uma maneira particular.

2) Isso seria suficiente para justificar uma mudança de carreira?

  • Não tocando neste.

3) Qual é a importância do código compreensível neste setor?

  • Extremamente. Se o código não for compreensível, você não pode ter certeza do que está fazendo nem do que NÃO está fazendo. Em outras palavras, você não tem como saber se está funcionando corretamente.

4a) Os exemplos C:

  • Os exemplos citados de C não são equivalentes. O segundo caso dará os mesmos resultados que o primeiro se e somente se b estiver restrito aos valores de 0 e 1. No entanto, se você lançar um valor de, digamos, 173,406,926 para b , os resultados NÃO serão correspondentes.

4b) Existe alguma desculpa para empregar o último exemplo?

  • Sim. Muitos processadores empregam predição de ramificação e pipelines. O custo de uma ramificação incorretamente prevista pode introduzir atrasos inaceitáveis. Para obter uma resposta mais determinista, o uso de bit twiddling pode ser usado para suavizar as coisas.

  • Minha opinião pessoal sobre o segundo exemplo é que é desnecessariamente complicada e deve ser retrabalhada para maior clareza. Além disso, eu não gosto da multiplicação como (dependendo da arquitetura) pode ser lento quando comparado a outras operações. Muito provavelmente, eu preferiria ver algo da forma:

    return b ? 42 : 7;
    
  • No entanto, dependendo da circunstância (E se puder ser demonstrado que ela forneceu melhores resultados substancialmente melhores em categorias críticas), eu poderia aceitar uma macro com um nome descritivo apropriado ao revisar o código. Por exemplo:

    /*
     * APPROPRIATE_MACRO_NAME - use (x) if (condition) is 1, (y) if (condition) is 0
     *
     * Parameter Restrictions:
     * (condition) - must be either 1 or 0
     * (x) and (y) are of the same integer type
     *
     * This macro generates code that avoids branching (and thus incorrectly
     * predicted branches).  Its use should be restricted to <such and such>
     * because <why and why>.
     */
    #define APPROPRIATE_MACRO_NAME(condition,x,y)  \
        (((condition - 1) & ((y) - (x))) + (x))
    

Espero que isso ajude.

    
por 28.02.2015 / 02:46
fonte
38

O segundo código não retorna 42 ou 7.

for b = 1:
  (1 * 42) | (~(1 - 1) * 7)
  42 | (~(0) * 7) 
  42 | (-1 * 7) 
  42 | -7
  -5

for b = 0:
  (0 * 42) | (~(0 - 1) * 7)
  0 | (~(-1) * 7) 
  0 | (0 * 7) 
  0 | 0
  0

E, no entanto, quando você postou, achou que sim, e é exatamente por isso que você deve evitar um código complicado.

No entanto, pegue algum código "correto", por exemplo, como:

return ((b - 1) & (7 ^ 42)) ^ 42;

Existem duas razões pelas quais posso pensar que pode ser útil. - A arquitetura para a qual você está escrevendo não suporta instruções de ramificação ou predicadas. - A arquitetura para a qual você está escrevendo possui um pipeline que não funciona após uma operação de ramificação ou que possui um custo proibitivo associado a uma falha de predição de filial.

Nesse caso, você escreveria algo ao longo destas linhas:

/* 
   This code is equivalent to:

   if (b)
      return 42;
   else
      return 7;

   when b=0 or b=1

   But does not include a branch instruction since a branch prediction
   miss here would cause an unacceptable impact to performance. 
*/

return ((b - 1) & (7 ^ 42)) ^ 42;

Se, no entanto, você vir apenas um código como esse, sem explicação ou lógica, provavelmente será um sinal de ofuscação do código-fonte. A ofuscação do código fonte (em oposição à ofuscação de código binário, que muitos considerariam ter propósitos legítimos) tende a ser nefasta. Alguns programadores podem ser territoriais por vários motivos, às vezes por segurança no emprego e, às vezes, por maior controle sobre o que é feito e como as coisas são feitas por causa do ego, da insegurança ou da desconfiança. No entanto, é quase sempre contra os interesses dos colegas e do empregador. É responsabilidade de quem está no comando da equipe eliminar esse comportamento o mais cedo possível, seja construindo confiança mútua ou instilando medo, dependendo do estilo de gerenciamento do líder e a quais métodos o programador individual responde.

    
por 28.02.2015 / 06:00
fonte
6

As probabilidades são de que alguém escrevendo (b * 42) | (~(b - 1) * 7) seja alguém que saiba muito pouco sobre programação tentando fingir ser experiente / experiente / etc, ou alguém tentando sabotar um projeto (ou seja, eles são muito experientes / experientes / inteligentes e quer segurança no emprego).

O primeiro tipo de pessoa quer mostrar que sabe usar NOT, OR, ordem de operações, etc. Eles estão mostrando seu conhecimento, mas, infelizmente, eles estão escrevendo código que é muito menos eficiente, porque isso requer duas multiplicações, uma subtração, uma não e uma ou, que é claramente menos eficiente do que uma comparação, um par pula e um retorno.

Se for esse o caso, eles não merecem estar na indústria (ainda), mas sua demonstração prova que eles sabem lógica básica do computador e podem ser um recurso valioso um dia, depois de passarem showboating e começarem a escrever código eficiente . Além disso, há a possibilidade distinta de que b não seja necessariamente 0 ou 1, o que resultaria em um valor completamente diferente sendo retornado. Isso pode introduzir bugs sutis que podem ser difíceis de encontrar.

O segundo tipo de pessoa espera inserir códigos como esse (ou muitos outros tipos desonestos de código), de modo que as pessoas continuem fazendo perguntas sobre o código, então eles são considerados valiosos demais para serem perdidos. Esse tipo de sabotagem acabará prejudicando um projeto, e eles devem ser liberados imediatamente até aprenderem a lição e escreverem um código otimizado e fácil de ler. Existe também a possibilidade de que b não seja 1 ou 0, como antes, o que significa que ele retornará um valor diferente do esperado (42 ou 7), o que pode funcionar corretamente até que algum programador infeliz o altere de volta para if(b) ... else ... e código de repente pára de funcionar. Por exemplo, talvez este seja um gerador de pseudo-número disfarçado como uma declaração if muito cara.

O código legível é importante, assim como o código livre (tanto quanto prático) de erros lógicos como este. Qualquer pessoa que esteja gravando código a sério sabe o quanto isso é importante. Escreva um jogo totalmente funcional de Tic Tac Toe. Agora, defina esse código por um ano, depois volte a ele e tente ler o código. Se você escreveu de forma informal, sem considerar padrões de codificação, comentários, documentação, etc, você provavelmente nem reconheceria que o código que você escreveu foi digitado por você, muito menos como consertá-lo ou adicionar um novo recurso se algo foi quebrado ou precisava ser atualizado.

Quando vários desenvolvedores trabalham juntos, é ainda mais importante que o código seja legível, porque as chances são de que você não mantenha esse código. Você terá passado para outras coisas e outra pessoa terá que manter seu código. Por outro lado, você pode herdar outro projeto e esperará que os desenvolvedores deixem comentários e códigos limpos para você trabalhar. Programadores trabalhando em código confiável escrevem o código para ser sustentável, incluindo legibilidade e comentários.

O desempenho, embora também importante, não deve ser mais uma questão de legibilidade. No mínimo, se alguém usou o segundo bloco de código aqui, eu esperaria um longo bloco de comentários que explicasse claramente por que eles fizeram dessa maneira ao invés de uma maneira mais convencional. Nesse ponto, eu provavelmente decidiria substituí-lo por um código mais convencional se não houvesse uma boa razão para isso. Se, de fato, fosse uma bomba lógica, eu a reescreveria de uma maneira mais longa, entendendo que a função tem que ser assim para evitar bugs, junto com uma documentação clara do que ela realmente faz.

Como se constata, tenho certeza de que há um uso legítimo de algum problema especializado que coincidentemente parece precisar desse algoritmo preciso. Se assim for, porém, eu esperaria comentários explicando o algoritmo que usa essa lógica, e é melhor que seja para algo melhor do que um bloco if-else. Os dois únicos blocos if-else apropriados para o exemplo específico são: if(b) return 42; return 7; (else é opcional) e return b? 42: 7; (os operadores ternários estão bem para a lógica de ramificação pequena, a menos que os padrões de codificação proíbam completamente). Qualquer outra coisa é excessivamente complicada e deve ser reduzida a uma declaração mais simples.

Ocasionalmente me vejo escrevendo código "complicado" que desenvolvedores mais jovens não entendem imediatamente, e eu costumo comentar esse código para que eles possam entender por que ele foi escrito da maneira que foi. Às vezes, o código otimizado pode ser difícil de ler e, no entanto, é necessário, mas essas devem ser a exceção e não a regra.

Existe, coincidentemente, um lugar perfeitamente aceitável para código como este. Concursos de ofuscação. Nesse caso, eu reservaria o julgamento para a função até que eu determinasse que o código era apenas uma ramificação realmente esperta, ou se era um dispositivo mais desonesto para gerar números pseudo-aleatórios, o valor para PI (ou algum outro número mágico), ou talvez até mesmo um algoritmo de criptografia fraco.

    
por 28.02.2015 / 06:44
fonte
4

Se as ramificações em if / then / else forem um problema, provavelmente será mais fácil mudar para algo como:

static const int values[] = {6, 42};

return values[b!=0];

Isso realmente funciona e, embora alguns possam achá-lo marginalmente menos legível do que o if / then / else , certamente não deve ser uma obstrução perceptível para quem conhece C ou C ++.

Para quem deve pensar que se trata de um truque sujo, ou depende de uma verificação de tipo particularmente frouxa em um idioma específico: sim e não. Conforme escrito, o código depende de "false" convertendo para 0 e "true" para 1 em C e C ++.

A idéia básica, no entanto, pode ser aplicada igualmente bem em outras linguagens, incluindo aquelas que possuem sistemas de tipos substancialmente mais restritos. Por exemplo, em Pascal, a mesma ideia básica seria:

var
    values : array [boolean] of Integer;

(* ... *)
values[false] = 6;
values[true] = 42;

(* ... *)
f = values[b<>0];

O Manual do Usuário em Pascal e Relatório (2 nd Edition) por Kathleen Jensen e Niklaus Wirth mostra o uso de um Booleano como um índice em uma matriz em um número de lugares, como §6.2.1 e §7. Embora o Pascal (como definido originalmente) não inclua a noção de variáveis inicializadas, se isso ocorresse, os inicializadores acabariam na mesma ordem em C e C ++ (como define o fato de que false < true ). / p>

A Ada usa uma sintaxe ligeiramente diferente, mas fornece o mesmo recurso básico:

Temp    : array(Boolean) of Integer := (false => 7, true=>42);

-- ...

Return_Value = Temp( 0 /= i);

Então, não estamos lidando com algo que é apenas um acidente de uma língua que usa uma verificação de tipo particularmente frouxa. Pascal e (especialmente) Ada são bem conhecidos por serem particularmente strongmente tipados, mas ambos suportam o mesmo constructo básico como C e C ++, e o fazem essencialmente da mesma maneira.

    
por 28.02.2015 / 04:20
fonte
3

Se eu fosse apresentado um código assim em uma revisão de código, teria duas perguntas a serem feitas:

  • Por que escolhemos escrever dessa maneira? Como a manipulação de bits como essa é usada para contornar algum tipo de gargalo de desempenho, podemos presumir que temos um gargalo que é retificado se empregarmos essa abordagem. Uma questão de acompanhamento imediata seria: "Como provamos que esse era um gargalo crítico?"

  • Temos algum tipo de estrutura de aceitação (testes de unidade, etc.) que prove que a abordagem otimizada é equivalente à abordagem não otimizada? Se nós não , é quando os alarmes disparam, mas eles não são tão altos quanto se poderia considerar.

Por fim, o código que você escreve deve ser maintainable . Código que é rápido, mas difícil de abordar quando ocorre um erro não é um bom código. Se eles pudessem satisfazer ambas as minhas preocupações acima, eu provavelmente deixaria ir com uma solicitação para adicionar a justificativa e sua forma equivalente, não-bit.

Em geral, uma revisão de código deve identificar e esclarecer essas deficiências; se não houvesse razão justificável para escrever o código dessa maneira, ou o código simplesmente não estivesse correto (como alguns outros apontaram aqui), não há razão para ter escrito dessa forma em primeiro lugar, e isso deve ser corrigido . Se esta é uma tendência contínua; isto é, um desenvolvedor continua a escrever código que seja eficiente, mas horrivelmente ofuscado, nesse ponto, o líder ou a gerência sênior deve intervir e reiterar a importância de um código claro.

    
por 01.03.2015 / 21:41
fonte
3

Algo como o seguinte seria um caminho para tornar a INTENÇÃO do código mais aparente:

manifoldPressureFloor = (b * 42) | (~(b - 1) * 7);

return manifoldPressureFloor;

manifoldPressureFloor é totalmente inventado, é claro que não tenho ideia do que o código original é realmente.

Mas sem algum tipo de explicação ou justificativa para o código ofuscado, o revisor de código e / ou programador de manutenção está na posição de ter nenhuma idéia clara do que o programador original realmente pretendeu realizar , por sua vez, torna quase impossível provar que o código realmente funciona (que ele realmente faz o que é suposto fazer). E isso torna a programação de manutenção mais dolorosa e muito mais provável de apresentar erros.

Eu não acredito (sem qualificações) que o código pode ser completamente auto-comentador. Contudo; Eu acredito totalmente que é possível inclinar-se strongmente nessa direção.

Se houver alguma razão incomum (ou borda, desempenho ou outra até agora não especificada) porque a sintaxe (b * 42) | (~(b - 1) * 7) é justificada, o revisor de código não foi capaz de compreender facilmente esse fato porque a INTENÇÃO do código não é facilmente decifrada e aparentemente não houve comentários explicando por que isso foi feito.

O código que é inteligente apenas para ser inteligente deve ser evitado. Um dos principais objetivos de escrever um código claro é garantir a compreensão humana e impedir que os bugs sejam introduzidos no futuro. Um código inteligente que não é claramente compreensível é uma bomba-relógio. Mesmo que funcione hoje, ele vai parar de funcionar depois da manutenção do código amanhã ou daqui a um ano. E os bugs serão esotéricos e difíceis de encontrar e consertar.

Como alguém que não seja o programador original pode provar que o código funciona se a intenção e a sintaxe não estiverem claras? Como pode até mesmo o programador original provar que o código funciona nesse caso?

Os comentários devem ser exigidos em um código de caso de borda inteligente e estridente. Mas é melhor que o código seja simples o suficiente para não exigir os comentários. Esses comentários também acabam desatualizados ou irrelevantes se o código for atualizado e o programador de manutenção não atualizar ou remover os comentários. Os programadores atualizam rotineiramente o código sem atualizar os comentários. Eles não devem , apenas fazer .

Todos esses fatores levam à conclusão de que este programador precisa abordar as questões de ser capaz de provar que o código funciona, tornar o código compreensível por outros seres humanos e tornar o código robusto . Código que é altamente suscetível à introdução de novos bugs durante a manutenção é frágil. Nenhum empregador quer pagar muito dinheiro pelo desenvolvimento de códigos frágeis e sujeitos a falhas.

O código frágil, cujo comportamento é improvável, é muito mais caro do que código limpo, claro, comprovável (testável), fácil de compreender e fácil de manter. O programador está sendo pago por outra pessoa para ser um profissional e produzir um produto razoavelmente limpo, robusto e sustentável a um custo razoável. Certo?

Então, em conclusão, o programador que criou esse código é obviamente inteligente e, portanto, cheio de potencial. Mas as questões de ser capaz de provar a correção do código, tornam o código robusto e compreensível, precisam ser levadas a sério. Se essa mudança pode ocorrer, o programador provavelmente vale a pena continuar. Se não, e o programador insiste em continuar fazendo as coisas dessa maneira, pode ser difícil justificar mantê-los na equipe, porque eles quase certamente lhe custarão mais do que o farão a longo prazo. E esse é o problema real, não é?

IMHO.

    
por 28.02.2015 / 06:46
fonte
2

A substituição de if-s por expressões aritméticas / lógicas às vezes é necessária quando é necessária uma longa tubulação do processador. Isso faz com que o código execute sempre as mesmas instruções, seja qual for a condição, tornando-a mais adequada para a paralelização.

Dito isto, a amostra fornecida está errada, uma vez que as duas amostras não são equivalentes:

if (b)
  return 42;
else
  return 7;

pode ser return ((b&1)*42)|((1-(b&1)))*7

O programador da amostra OP pode ter feito uma eliminação se errada, ou o próprio OP pode ter interpretado erroneamente as intenções do programador ao adivinhar a opção if tradicional de maneira errada. Dizer qual dos dois está correto é difícil não saber qual foi o requisito.

Note que a expressão não é tão "obscura": é apenas uma combinação linear na forma P*(t)+Q*(1-t) (com t = 0..1) que todo programador deve estar ciente, já que é a base da maioria de álgebra linear.

A única coisa que eu posso entender é que entre o codificador e o revisor há uma base cultural diferente sobre o processamento paralelo e a álgebra linear. Este IMHO, não faz um superior ao outro, se a correção é comprovada.

    
por 28.02.2015 / 22:51
fonte
1

Além dos excelentes pontos feitos por Sparky (itens 1 e 3) e Donscarletti, este post me leva a outro que eu respondi há muito tempo: Como eu documentei meu código? .

Muitas pessoas são ou se chamam de programadores, algumas são boas, poucas são excelentes. Assim como em muitas outras esferas da vida. Você pode decidir julgar aqueles que parecem menos bons do que você ou menos bons do que o que você esperaria que fossem (não muito útil, perda de tempo), você pode tentar ajudá-los (ótimo) ou coagi-los a fazer melhor ( você pode não ter escolha), ou ignorá-los e simplesmente fazer o seu melhor (você pode não ter escolha, às vezes é o melhor caminho). Seja qual for a sua escolha de ação, geralmente depende de muitos fatores além da capacidade técnica simples.

    
por 02.03.2015 / 03:58
fonte