Meu chefe me pede para parar de escrever pequenas funções e fazer tudo no mesmo loop

Eu li um livro chamado Clean Code, de Robert C. Martin. Neste livro, vi muitos métodos para limpar código, como escrever pequenas funções, escolher nomes com cuidado, etc. Parece, de longe, o livro mais interessante sobre código limpo que já li. No entanto, hoje meu chefe não gostou da maneira como escrevi o código depois de ler este livro.

Seus argumentos foram

• Escrever pequenas funções é uma dor, porque obriga a passar para cada função pequena para ver o que o código está fazendo.
• Coloque tudo em um loop grande principal, mesmo que o loop principal tenha mais de 300 linhas, é mais rápido ler.
• Escreva apenas pequenas funções se você precisar duplicar o código.
• Não escreva uma função com o nome do comentário, coloque sua complexa linha de código (3-4 linhas) com um comentário acima; Da mesma forma, você pode modificar o código com falha diretamente

Isso é contra tudo o que li. Como você costuma escrever código? Um grande loop principal, sem pequenas funções?

A linguagem que eu uso é principalmente Javascript. Eu realmente tenho dificuldades para ler agora, pois excluí todas as minhas pequenas funções claramente nomeadas e coloco tudo em um grande loop. No entanto, meu chefe gosta dessa maneira.

Um exemplo foi:

// The way I would write it
if (isApplicationInProduction(headers)) {
  phoneNumber = headers.resourceId;
} else {
  phoneNumber = DEV_PHONE_NUMBER;
}

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

// The way he would write it
// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;

No livro que li, por exemplo, os comentários são considerados como falha na gravação de código limpo, porque são obsoletos se você escrever pequenas funções e geralmente levar a comentários não atualizados (você modifica seu código e não o comentário). No entanto, o que faço é excluir o comentário e escrever uma função com o nome do comentário.

Bem, gostaria de alguns conselhos, de que maneira / prática é melhor escrever código limpo?

Tomando os exemplos de código primeiro. Você favorece:

if (isApplicationInProduction(headers)) {
  phoneNumber = headers.resourceId;
} else {
  phoneNumber = DEV_PHONE_NUMBER;
}

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

E seu chefe escreveria como:

// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;

Na minha opinião, ambos têm problemas. Ao ler seu código, meu pensamento imediato foi "você pode substituí-lo ifpor uma expressão ternária". Então li o código do seu chefe e pensei "por que ele substituiu sua função por um comentário?".

Sugiro que o código ideal esteja entre os dois:

phoneNumber = isApplicationInProduction(headers) ? headers.resourceId : DEV_PHONE_NUMBER;

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

Isso oferece o melhor dos dois mundos: uma expressão de teste simplificada e o comentário é substituído por um código testável.

Em relação às opiniões do seu chefe sobre o design de código:

Escrever pequenas funções é uma dor, porque obriga a passar para cada uma das pequenas funções para ver o que o código está fazendo.

Se a função for bem nomeada, este não é o caso. isApplicationInProductioné evidente e não deve ser necessário examinar o código para ver o que ele faz. De fato, o oposto é verdadeiro: examinar o código revela menos sobre a intenção do que o nome da função (e é por isso que seu chefe precisa recorrer a comentários).

Coloque tudo em um loop grande principal, mesmo que o loop principal tenha mais de 300 linhas, é mais rápido ler

Pode ser mais rápido varrer, mas para realmente "ler" o código, você precisa executá-lo efetivamente em sua cabeça. Isso é fácil com pequenas funções e é muito, muito difícil com métodos com centenas de linhas.

Escreva apenas pequenas funções se você precisar duplicar o código

Discordo. Como mostra seu exemplo de código, funções pequenas e bem nomeadas melhoram a legibilidade do código e devem ser usadas sempre que, por exemplo, você não estiver interessado no "como", apenas no "o quê" de uma parte da funcionalidade.

Não escreva uma função com o nome do comentário, coloque sua complexa linha de código (3-4 linhas) com um comentário acima. Assim, você pode modificar o código com falha diretamente

Eu realmente não consigo entender o raciocínio por trás deste, assumindo que realmente seja sério. É o tipo de coisa que eu esperaria ver escrita em paródia pela conta do twitter do The Expert Beginner . Os comentários têm uma falha fundamental: eles não são compilados / interpretados e, portanto, não podem ser testados em unidade. O código é modificado e o comentário é deixado sozinho e você acaba não sabendo o que é certo.

É difícil escrever código de auto-documentação e documentos às vezes necessários (mesmo na forma de comentários). Mas a visão do "tio Bob" de que os comentários são uma falha de codificação é verdadeira com muita frequência.

Faça seu chefe ler o livro Código Limpo e tente resistir a tornar seu código menos legível apenas para satisfazê-lo. No final das contas, se você não pode convencê-lo a mudar, você precisa se alinhar ou encontrar um novo chefe que possa codificar melhor.

Existem outros problemas

Nenhum dos códigos é bom, porque ambos basicamente incham o código com um caso de teste de depuração . E se você quiser testar mais coisas por qualquer motivo?

phoneNumber = DEV_PHONE_NUMBER_WHICH_CAUSED_PROBLEMS_FOR_CUSTOMERS;

ou

phoneNumber = DEV_PHONE_NUMBER_FROM_OTHER_COUNTRY;

Deseja adicionar ainda mais ramos?

O problema significativo é que você basicamente duplica parte do seu código e, portanto, não está realmente testando o código real. Você escreve o código de depuração para testar o código de depuração, mas não o código de produção. É como criar parcialmente uma base de código paralela.

Você está discutindo com seu chefe sobre como escrever códigos ruins de maneira mais inteligente. Em vez disso, você deve corrigir o problema inerente ao próprio código.

Injeção de dependência

É assim que seu código deve ficar:

phoneNumber = headers.resourceId;

Não há ramificação aqui, porque a lógica aqui não possui ramificação. O programa deve puxar o número de telefone do cabeçalho. Período.

Se você deseja obter DEV_PHONE_NUMBER_FROM_OTHER_COUNTRYcomo resultado, deve inseri-lo headers.resourceId. Uma maneira de fazer isso é simplesmente injetar um headersobjeto diferente para os casos de teste (desculpe se este não for um código adequado, minhas habilidades em JavaScript estão um pouco enferrujadas):

function foo(headers){
    phoneNumber = headers.resourceId;
}

// Creating the test case
foo({resourceId: DEV_PHONE_NUMBER_FROM_OTHER_COUNTRY});

Supondo que headersseja parte de uma resposta que você recebe de um servidor: Idealmente, você tem um servidor de teste inteiro que fornece headersvários tipos para fins de teste. Dessa forma, você testa o código de produção real como está e não um código meio duplicado que pode ou não funcionar como o código de produção.

Não há resposta "certa" ou "errada" para isso. No entanto, vou oferecer minha opinião com base em 36 anos de experiência profissional no design e desenvolvimento de sistemas de software ...

1. Não existe "código de auto-documentação". Por quê? Porque essa afirmação é completamente subjetiva.
2. Comentários nunca são falhas. O que é uma falha é um código que não pode ser entendido sem comentários.
3. 300 linhas de código ininterruptas em um bloco de código é um pesadelo de manutenção e altamente propenso a erros. Esse bloco é fortemente indicativo de mau design e planejamento.

Falando diretamente com o exemplo que você forneceu ... Colocar isApplicationInProduction()em sua própria rotina é a coisa mais inteligente a ser feita. Hoje esse teste é simplesmente uma verificação de "cabeçalhos" e pode ser manipulado em um ?:operador ternário ( ). Amanhã, o teste pode ser muito mais complexo. Além disso, "headers.resourceId" não tem uma relação clara com o "status da produção" do aplicativo; Eu argumentaria que um teste para esse status precisa ser dissociado dos dados subjacentes; uma sub-rotina fará isso e um ternário não. Além disso, um comentário útil seria por que resourceId é um teste para "em produção".

Cuidado para não exagerar nas "pequenas funções claramente nomeadas". Uma rotina deve encapsular uma ideia mais do que "apenas código". Apoio a sugestão de amon phoneNumber = getPhoneNumber(headers)e acrescento que getPhoneNumber()deve fazer o teste "status da produção" comisApplicationInProduction()

"As entidades não devem ser multiplicadas além da necessidade."

- Navalha de Occam

O código deve ser o mais simples possível. Os erros gostam de se esconder entre a complexidade, porque são difíceis de detectar por aí. Então, o que torna o código simples?

Pequenas unidades (arquivos, funções, classes) são uma boa idéia . Unidades pequenas são fáceis de entender porque há menos coisas que você precisa entender ao mesmo tempo. Os seres humanos normais podem apenas manipular ~ 7 conceitos de cada vez. Mas o tamanho não é medido apenas em linhas de código . Eu posso escrever o mínimo de código possível "jogando golfe" o código (escolhendo nomes curtos de variáveis, pegando atalhos "inteligentes", quebrando o máximo de código possível em uma única linha), mas o resultado final não é simples. Tentar entender esse código é mais como engenharia reversa do que leitura.

Uma maneira de encurtar uma função é extrair várias funções auxiliares. Essa pode ser uma boa idéia quando extrai uma parte independente da complexidade . Isoladamente, essa parte da complexidade é muito mais simples de gerenciar (e testar!) Do que quando incorporada a um problema não relacionado.

Mas toda chamada de função tem uma sobrecarga cognitiva : não preciso apenas entender o código dentro do meu pedaço de código atual, também preciso entender como ele interage com o código externo . Eu acho justo dizer que a função que você extraiu introduz mais complexidade na função do que extrai . É isso que seu chefe quer dizer com “ pequenas funções [são] uma dor, porque obriga você a passar para cada função pequena para ver o que o código está fazendo. "

Às vezes, funções chatas longas podem ser incrivelmente simples de entender, mesmo quando têm centenas de linhas. Isso costuma ocorrer no código de inicialização e configuração, por exemplo, ao criar uma GUI manualmente, sem um editor de arrastar e soltar. Não há complexidade independente que você possa extrair razoavelmente. Mas se a formatação é legível e há alguns comentários, não é realmente difícil acompanhar o que está acontecendo.

Existem muitas outras métricas de complexidade: O número de variáveis ​​em um escopo deve ser o menor possível. Isso não significa que devemos evitar variáveis. Isso significa que devemos restringir cada variável ao menor escopo possível onde for necessário. As variáveis ​​também se tornam mais simples se nunca mudarmos o valor que elas contêm.

Uma métrica muito importante é a complexidade ciclomática (complexidade de McCabe). Ele mede o número de caminhos independentes através de um pedaço de código. Esse número cresce exponencialmente a cada condicional. Cada condicional ou loop dobra o número de caminhos. Há evidências para sugerir que uma pontuação de mais de 10 pontos é muito complexa. Isso significa que uma função muito longa que talvez tenha uma pontuação de 5 talvez seja melhor do que uma função muito curta e densa com uma pontuação de 25. Podemos reduzir a complexidade extraindo o fluxo de controle em funções separadas.

Seu condicional é um exemplo de um pedaço de complexidade que pode ser extraído inteiramente:

function bigFatFunction(...) {
  ...
  phoneNumber = getPhoneNumber(headers);
  ...
}

...

function getPhoneNumber(headers) {
  return headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;
}

Isso ainda está muito próximo de ser útil. Não tenho certeza se isso diminui substancialmente a complexidade porque esse condicional não é muito condicional . Na produção, sempre seguirá o mesmo caminho.

A complexidade nunca pode desaparecer. Só pode ser embaralhado. Muitas coisas pequenas são mais simples do que poucas coisas grandes? Isso depende muito da circunstância. Geralmente, há alguma combinação que parece certa. Encontrar esse compromisso entre diferentes fatores de complexidade requer intuição e experiência, e um pouco de sorte.

Saber escrever funções muito pequenas e funções muito simples é uma habilidade útil, porque você não pode fazer uma escolha sem conhecer as alternativas. Seguir cegamente regras ou práticas recomendadas sem pensar em como elas se aplicam à situação atual leva a resultados médios na melhor das hipóteses, programação de cultos de carga na pior .

É aí que eu discordo do seu chefe. Seus argumentos não são inválidos, mas o livro do Código Limpo também não está errado. Provavelmente é melhor seguir as orientações de seu chefe, mas o fato de você estar pensando nesses problemas, tentando encontrar uma maneira melhor, é muito promissor. À medida que você ganha experiência, será mais fácil encontrar uma boa fatoração para o seu código.

(Nota: esta resposta se baseia em partes das reflexões da postagem do blog Reasonable Code no The Whiteboard de Jimmy Hoffa , que fornece uma visão de alto nível sobre o que simplifica o código.)

O estilo de programação de Robert Martin é polarizador. Você encontrará muitos programadores, mesmo os mais experientes, que encontram muitas desculpas porque dividir "tanto" é demais e por que manter as funções um pouco maiores é "o melhor caminho". No entanto, a maioria desses "argumentos" costuma ser uma expressão de falta de vontade de mudar velhos hábitos e aprender algo novo.

Não os escute!

Sempre que você puder salvar um comentário, refatorando um pedaço de código para uma função separada com um nome expressivo, faça-o - provavelmente irá melhorar seu código. Você não foi tão longe quanto Bob Martin o faz em seu livro de códigos limpo, mas a grande maioria do código que vi no passado que causava problemas de manutenção continha funções muito grandes, não muito pequenas. Portanto, tentar escrever funções menores com nomes autoexplicativos é o que você deve tentar.

As ferramentas de refatoração automática tornam mais fácil, simples e seguro extrair métodos. E, por favor, não leve a sério as pessoas que recomendam funções de escrita com> 300 linhas - essas pessoas definitivamente não estão qualificadas para lhe dizer como você deve codificar.

No seu caso: você deseja um número de telefone. Ou é óbvio como você obteria um número de telefone e depois escreveria o código óbvio. Ou não é óbvio como você obteria um número de telefone e depois escreveria um método para ele.

No seu caso, não é óbvio como obter o número de telefone, então você escreve um método para ele. A implementação não é óbvia, mas é por isso que você a coloca em um método separado, para que você tenha que lidar com isso apenas uma vez. Um comentário seria útil porque a implementação não é óbvia.

O método "isApplicationInProduction" é inútil. A chamada a partir do método getPhonenumber não torna a implementação mais óbvia e apenas torna mais difícil descobrir o que está acontecendo.

Não escreva pequenas funções. Escreva funções que tenham um objetivo bem definido e atendam a esse objetivo bem definido.

PS. Não gosto da implementação. Ele pressupõe que a ausência de número de telefone significa que é uma versão do desenvolvedor. Portanto, se o número de telefone estiver ausente na produção, você não apenas o manipula, como também substitui um número de telefone aleatório. Imagine que você tem 10.000 clientes e 17 não tem um número de telefone e está com problemas na produção. Se você está em produção ou desenvolvimento, deve ser verificado diretamente, não derivado de outra coisa.

Mesmo ignorando o fato de que nenhuma implementação é tão boa assim, eu observaria que isso é essencialmente uma questão de gosto, pelo menos no nível de abstração de funções triviais de uso único.

O número de linhas não é uma métrica útil na maioria dos casos.

300 (ou até 3000) linhas de código puramente seqüencial e trivialmente trivial (instalação ou algo parecido) raramente são um problema (mas pode ser melhor gerado automaticamente ou como tabela de dados ou algo assim), 100 linhas de loops aninhados com muitas complicações condições de saída e matemática, como você pode encontrar na eliminação gaussiana ou na inversão de matriz, ou isso pode ser muito difícil de seguir facilmente.

Para mim, eu não escreveria uma função de uso único, a menos que a quantidade de código necessária para declarar a coisa fosse muito menor que a quantidade de código que forma a implementação (a menos que eu tivesse razões como dizer querer poder facilmente fazer a injeção de falhas). Um único condicional raramente se encaixa nesse projeto.

Agora eu venho de um mundo pequeno e incorporado, onde também temos que considerar coisas como profundidade da pilha e sobrecarga de chamada / retorno (que novamente argumenta contra os tipos de pequenas funções que parecem ser defendidas aqui), e isso pode estar influenciando meu design decisões, mas se eu visse a função original em uma revisão de código, receberia uma chama antiga da usenet em resposta.

Gosto do design é difícil de ensinar e só realmente traz experiência, não tenho certeza de que possa ser reduzido a regras sobre o comprimento das funções, e até a complexidade ciclomática tem seus limites como métrica (às vezes as coisas são complicadas, mas você as enfrenta).
Isso não quer dizer que o código limpo não discuta algumas coisas boas, mas sim, e essas coisas devem ser pensadas, mas o costume local e o que a base de código existente faz também deve ter peso.

Esse exemplo em particular parece-me estar escolhendo detalhes triviais; eu ficaria mais preocupado com coisas de nível muito mais alto, pois isso importa muito mais com a capacidade de entender e depurar um sistema facilmente.

Não coloque tudo em um loop grande, mas também não faça isso com muita frequência:

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

O problema com o grande loop é que é realmente difícil ver sua estrutura geral quando se estende por várias telas. Portanto, tente remover pedaços grandes, idealmente pedaços que tenham uma única responsabilidade e sejam reutilizáveis.

O problema com a pequena função acima é que, embora a atomicidade e a modularidade sejam geralmente boas, isso pode ser levado longe demais. Se você não vai reutilizar a função acima, isso prejudica a legibilidade e a manutenção do código. Para detalhar os detalhes, você precisa pular para a função em vez de poder ler os detalhes em linha, e a chamada da função ocupa pouco menos espaço do que o detalhe em si.

Claramente, há um equilíbrio entre métodos que fazem muito e métodos que fazem muito pouco . Eu nunca quebraria uma função minúscula como acima, a menos que fosse chamada de mais de um lugar, e mesmo assim pensaria duas vezes sobre ela, porque a função simplesmente não é tão substancial em termos de introdução de nova lógica e quase não garante ter sua própria existência.

Parece que o que você realmente deseja é este:

phoneNumber = headers.resourceId || DEV_PHONE_NUMBER

Isso deve ser auto-explicativo para quem lê: defina phoneNumbercomo resourceIdse estiver disponível ou padrão como DEV_PHONE_NUMBERse não estiver.

Se você realmente deseja definir essa variável apenas na produção, deve ter outro método utilitário mais abrangente para todo o aplicativo (que não exija parâmetros) para determinar de onde você está executando. Ler os cabeçalhos dessa informação não faz sentido.

Deixe-me ser franco: parece-me que seu ambiente (linguagem / estrutura / design de classe etc.) não é realmente adequado para código "limpo". Você está misturando todos os tipos possíveis de coisas em algumas linhas de código que realmente não devem estar muito próximas. Que negócio uma única função possui sabendo que resourceId==undefsignifica que você não está em produção, que está usando um número de telefone padrão em sistemas que não são de produção, que o resourceId é salvo em alguns "cabeçalhos" e assim por diante? Eu assumo que headerssão cabeçalhos HTTP, então você ainda deixa a decisão sobre qual ambiente você está para o usuário final?

A fatoração de partes únicas disso em funções não ajudará muito com esse problema subjacente.

Algumas palavras-chave para procurar:

• dissociação
• coesão
• Injeção de dependência

Você pode conseguir o que deseja (em outros contextos) com zero linhas de código, alterando as responsabilidades do código e usando estruturas modernas (que podem ou não existir para o seu ambiente / linguagem de programação).

Pela sua descrição ("300 linhas de código em uma função 'principal'"), mesmo a palavra "função" (em vez de método) me faz supor que não há sentido no que você está tentando alcançar. Nesse ambiente de programação da velha escola (ou seja, programação imperativa básica com pouca estrutura, certamente nenhuma classe significativa, nenhum padrão de estrutura de classe como MVC ou algo assim), não há realmente muito sentido em fazer alguma coisa . Você nunca sairá do poço sem mudanças fundamentais. Pelo menos seu chefe parece permitir que você crie funções para evitar a duplicação de código, esse é um bom primeiro passo!

Eu sei tanto o tipo de código quanto o tipo de programador que você está descrevendo muito bem. Francamente, se fosse um colega de trabalho, meu conselho seria diferente. Mas, como é seu chefe, é inútil você brigar por isso. Não apenas o seu chefe pode anular você, mas suas adições ao código levarão a um código pior se você fizer o que você faz parcialmente e o seu chefe (e provavelmente outras pessoas) continuar como antes. Pode ser melhor para o resultado final se você se adaptar ao estilo de programação (apenas enquanto trabalha nessa base de código específica, é claro) e tentar, nesse contexto, tirar o melhor proveito dela.

"Limpo" é um objetivo na escrita de código. Não é o único objetivo. Outro objetivo digno é a colocalidade . Em termos informais, a colocalidade significa que as pessoas que tentam entender seu código não precisam pular para ver o que você está fazendo. Usar uma função bem nomeada em vez de uma expressão ternária pode parecer uma coisa boa, mas dependendo de quantas funções você tem e de onde elas estão localizadas, essa prática pode se tornar um incômodo. Não posso dizer se você ultrapassou essa linha, exceto para dizer que, se as pessoas estão reclamando, você deve ouvir, principalmente se essas pessoas têm uma opinião sobre seu status de emprego.

O uso de pequenas funções é, em geral, uma boa prática. Mas, idealmente, acredito que a introdução de uma função deve separar grandes pedaços lógicos ou reduzir o tamanho geral do código secando-o. O exemplo que você deu a ambos torna o código mais longo e requer mais tempo para um desenvolvedor ler, enquanto a alternativa curta não explica que o "resourceId"valor está presente apenas na produção. Algo simples como esse é fácil de esquecer e confuso ao tentar trabalhar com ele, principalmente se você ainda é novo na base de código.

Não vou dizer que você deve usar absolutamente um ternário, algumas pessoas com quem trabalhei preferem um pouco mais if () {...} else {...}, é principalmente uma escolha pessoal. Costumo preferir uma abordagem de "uma linha faz uma coisa", mas basicamente me ateria ao que a base de código normalmente usa.

Ao usar o ternário, se a verificação lógica tornar a linha muito longa ou complicada, considere criar uma variável / s bem nomeada para manter o valor.

// NOTE "resourceId" not present in dev build, use test data
let isProduction = 'resourceId' in headers;
let phoneNumber = isProduction ? headers.resourceId : DEV_PHONE_NUMBER;

Eu também gostaria de dizer que, se a base de código estiver estendendo-se para as funções de 300 linhas, ela precisará de alguma subdivisão. Mas eu aconselho o uso de traços um pouco mais amplos.

O exemplo de código que você deu, seu chefe ESTÁ CORRETO. Uma única linha clara é melhor nesse caso.

Em geral, dividir a lógica complexa em partes menores é melhor para legibilidade, manutenção de código e a possibilidade de que as subclasses tenham um comportamento diferente (mesmo que apenas um pouco).

Não ignore as desvantagens: sobrecarga da função, obscurecimento (a função não faz o que os comentários e o nome da função implicam), lógica complexa de espaguete, o potencial de funções mortas (ao mesmo tempo foram criadas para um propósito que não é mais chamado).

Eu posso pensar em pelo menos dois argumentos em favor de funções longas:

• Isso significa que você tem muito do contexto em torno de cada linha. Uma maneira de formalizar isso: desenhe o gráfico de fluxo de controle do seu código. Em um vértice (~ = linha) entre a entrada e a saída da função, você conhece todas as arestas recebidas. Quanto mais longa a função, mais existem vértices.

• Muitas funções pequenas significam que há um gráfico de chamadas maior e mais complexo. Escolha uma linha aleatória em uma função aleatória e responda à pergunta "em que contexto (s) essa linha é executada?" Isso se torna mais difícil, quanto maior e mais complexo for o gráfico de chamadas, porque é necessário observar mais vértices nesse gráfico.

Também existem argumentos contra funções longas - a testabilidade por unidade vem à mente. Use a sua experiência ao escolher entre um e outro.

Nota: não estou dizendo que seu chefe está certo, apenas que a perspectiva dele pode não ser completamente desprovida de valor.

Acho que minha opinião é que o bom parâmetro de otimização não é o tamanho da função. Eu acho que um desiderata mais útil para pensar em termos de é o seguinte: todos os demais sendo iguais, é preferível poder ler fora do código uma descrição de alto nível da lógica de negócios e da implementação. (Os detalhes de implementação de baixo nível sempre podem ser lidos se você encontrar o bit de código relevante.)

Comentando a resposta de David Arno :

Escrever pequenas funções é uma dor, porque obriga a passar para cada uma das pequenas funções para ver o que o código está fazendo.

Se a função for bem nomeada, este não é o caso. isApplicationInProduction é evidente e não deve ser necessário examinar o código para ver o que ele faz. De fato, o oposto é verdadeiro: examinar o código revela menos sobre a intenção do que o nome da função (e é por isso que seu chefe precisa recorrer a comentários).

O nome evidencia o que significa o valor de retorno , mas não diz nada sobre os efeitos da execução do código (= o que o código faz ). Os nomes (apenas) transmitem informações sobre a intenção , o código transmite informações sobre o comportamento (a partir do qual partes da intenção às vezes podem ser inferidas).

Às vezes você quer um, às vezes o outro, então essa observação não cria uma regra de decisão universalmente válida unilateral.

Coloque tudo em um loop grande principal, mesmo que o loop principal tenha mais de 300 linhas, é mais rápido ler

Pode ser mais rápido varrer, mas para realmente "ler" o código, você precisa executá-lo efetivamente em sua cabeça. Isso é fácil com pequenas funções e é muito, muito difícil com métodos com centenas de linhas.

Concordo que você precisa executá-lo em sua cabeça. Se você tem 500 linhas de funcionalidade em uma grande função vs. em muitas pequenas funções, não está claro para mim por que isso fica mais fácil.

Suponha o caso extremo de 500 linhas de código de efeito colateral linear e você deseja saber se o efeito A ocorre antes ou depois do efeito B. No grande caso de função, use Page Up / Down para localizar duas linhas e depois compare números de linha. No caso de muitas funções pequenas, você deve se lembrar de onde os efeitos ocorrem na árvore de chamadas e, se esqueceu, deve gastar um tempo não trivial redescobrindo a estrutura dessa árvore.

Ao percorrer a árvore de chamadas das funções de suporte, você também enfrenta o desafio de determinar quando passou da lógica de negócios para os detalhes da implementação. Afirmo sem evidência * que quanto mais simples o gráfico de chamadas, mais fácil é fazer essa distinção.

(*) Pelo menos eu sou honesto sobre isso ;-)

Mais uma vez, acho que ambas as abordagens têm pontos fortes e fracos.

Escreva apenas pequenas funções se você precisar duplicar o código

Discordo. Como mostra seu exemplo de código, funções pequenas e bem nomeadas melhoram a legibilidade do código e devem ser usadas sempre que [por exemplo] você não estiver interessado no "como", apenas no "o quê" de uma parte da funcionalidade.

Se você está interessado no "como" ou "o que" é uma função da finalidade para a qual você está lendo o código (por exemplo, obter uma idéia geral versus rastrear um bug). O objetivo para o qual você está lendo o código não está disponível durante a criação do programa e você provavelmente lerá o código para diferentes propósitos; decisões diferentes serão otimizadas para diferentes propósitos.

Dito isto, essa é a parte da visão do chefe com a qual eu provavelmente não concordo mais.

Não escreva uma função com o nome do comentário, coloque sua complexa linha de código (3-4 linhas) com um comentário acima. Assim, você pode modificar o código com falha diretamente

Eu realmente não consigo entender o raciocínio por trás deste, assumindo que realmente seja sério. [...] Os comentários têm uma falha fundamental: eles não são compilados / interpretados e, portanto, não podem ser testados em unidade. O código é modificado e o comentário é deixado sozinho e você acaba não sabendo o que é certo.

Os compiladores comparam apenas nomes para igualdade; eles nunca fornecem um MisleadingNameError. Além disso, como vários sites de chamada podem chamar uma determinada função pelo nome, às vezes é mais árduo e propenso a erros alterar um nome. Os comentários não têm esse problema. No entanto, isso é um pouco especulativo; para realmente resolver isso, provavelmente seria necessário ter dados sobre se os programadores têm maior probabilidade de atualizar comentários enganosos versus nomes enganosos, e eu não tenho isso.

Na minha opinião, o código correto para a funcionalidade necessária é:

phoneNumber = headers.resourceId || DEV_PHONE_NUMBER;

Ou se você deseja dividi-lo em uma função, provavelmente algo como:

phoneNumber = getPhoneNumber(headers);

function getPhoneNumber(headers) {
  return headers.resourceId || DEV_PHONE_NUMBER
}

Mas acho que você tem um problema mais fundamental com o conceito de "em produção". O problema com sua função isApplicationInProductioné que parece estranho que este seja o único local do sistema em que a "produção" é importante e que você sempre pode contar com a presença ou ausência do cabeçalho resourceId para informar. Deve haver um isApplicationInProductionmétodo geral ou getEnvironmentmétodo que verifique diretamente o ambiente. O código deve se parecer com:

function isApplicationInProduction() {
  process.env.NODE_ENV === 'production';
}

Você pode obter o número de telefone com:

phoneNumber = isApplicationInProduction() ? headers.resourceId : DEV_PHONE_NUMBER;

Apenas um comentário sobre dois dos pontos de bala

• Escrever pequenas funções é uma dor, porque obriga a passar para cada função pequena para ver o que o código está fazendo.
• Coloque tudo em um loop grande principal, mesmo que o loop principal tenha mais de 300 linhas, é mais rápido ler.

Muitos editores (por exemplo, IntelliJ) permitem que você pule para uma função / classe apenas clicando com a tecla Ctrl pressionada no uso. Além disso, muitas vezes você não precisa conhecer os detalhes de implementação de uma função para ler o código, tornando a leitura do código mais rápida.

Eu recomendo que você diga ao seu chefe; ele gostará da sua defesa e a verá como liderança. Seja educado.