Sou muito inteligente para ser lido por Jr devs? Demasiada programação funcional no meu JS? [fechadas]

No seu código, você fez várias alterações:

A atribuição de desestruturação
• para acessar campos no pages é uma boa mudança.
• extrair as funções parseFoo() , etc., é possivelmente uma boa mudança.
• introduzir um functor é… muito confuso.

Uma das partes mais confusas aqui é como você está misturando programação funcional e imperativa. Com o seu functor você não está realmente transformando dados, você está usando para passar uma lista mutável através de várias funções. Isso não parece uma abstração muito útil, já temos variáveis para isso. A coisa que possivelmente deveria ter sido abstraída - analisando apenas esse item, se existir - ainda está lá em seu código explicitamente, mas agora temos que pensar na esquina. Por exemplo, é um tanto não óbvio que parseFoo(foo) retornará uma função. O JavaScript não possui um sistema de tipo estático para notificá-lo se isso é legal, portanto, esse código é propenso a erros sem um nome melhor ( makeFooParser(foo) ?). Eu não vejo nenhum benefício neste ofuscação.

O que eu esperaria ver em vez disso:

if (foo) parseFoo(pages, foo);
if (bars) parseBar(pages, bars);
if (bazes) parseBaz(pages, bazes);
return pages;

Mas isso também não é ideal, porque não está claro no site de chamada que os itens serão adicionados à lista de páginas. Se, em vez disso, as funções de análise forem puras e retornar uma lista (possivelmente vazia) que possamos incluir explicitamente nas páginas, isso pode ser melhor:

pages.addAll(parseFoo(foo));
pages.addAll(parseBar(bars));
pages.addAll(parseBaz(bazes));
return pages;

Benefício adicionado: a lógica sobre o que fazer quando o item está vazio foi agora movida para as funções de análise individuais. Se isso não for apropriado, você ainda pode introduzir condicionais. A mutabilidade da lista pages agora é reunida em uma única função, em vez de propagá-la em várias chamadas. Evitar mutações não locais é uma parte muito maior da programação funcional do que abstrações com nomes engraçados como Monad .

Então, sim, seu código foi muito inteligente. Por favor, aplique sua esperteza para não escrever código inteligente, mas para encontrar maneiras inteligentes de evitar a necessidade de esperteza grosseira. Os melhores designs não parecem elegantes , mas parecem óbvios para quem os vê. E boas abstrações estão lá para simplificar a programação, não para adicionar camadas extras que eu tenho que desenredar em minha mente primeiro (aqui, descobrindo que o functor é equivalente a uma variável, e pode ser efetivamente eliminado).

Por favor: em caso de dúvida, mantenha seu código simples e estúpido (princípio KISS).

Se você está em dúvida, provavelmente é inteligente demais! O segundo exemplo introduz complexidade acidental com expressões como foo ? parseFoo(foo) : x => x e, em geral, o código é mais complexo, o que significa que é mais difícil de seguir.

O benefício pretendido, que você pode testar os pedaços individualmente, pode ser alcançado de uma forma mais simples, simplesmente invadindo funções individuais. E no segundo exemplo você combina as iterações separadas, de modo que você realmente obtém um isolamento menor .

Encontro um sinal de alerta em que você associa código simples e direto a "desenvolvedores novatos". Esta é uma mentalidade perigosa. Na minha experiência, é o oposto: os desenvolvedores novatos são propensos a códigos excessivamente complexos e inteligentes, porque requer mais experiência para poder ver a solução mais simples e clara.

O conselho contra "código inteligente" não é realmente sobre se o código usa ou não conceitos avançados que um principiante pode não entender. Em vez disso, trata-se de escrever código que seja mais complexo ou complicado do que o necessário . Isso torna o código mais difícil de ser seguido para todos , novatos e especialistas, e provavelmente também para você alguns meses depois.

Esta minha resposta chega um pouco atrasada, mas eu ainda quero entrar em cena. Só porque você está usando as mais recentes técnicas do ES6 ou usando o paradigma de programação mais popular não significa necessariamente que seu código está mais correto, ou o código desse júnior está errado. A programação funcional (ou qualquer outra técnica) deve ser usada quando for realmente necessária. Se você tentar encontrar a menor chance de criar as mais recentes técnicas de programação em cada problema, você sempre terá uma solução super projetada.

Dê um passo para trás e tente verbalizar o problema que você está tentando resolver por um segundo. Em essência, você quer apenas que uma função addPages transforme diferentes partes de apiData em um conjunto de pares de valores-chave e, em seguida, adicione todos eles em PagesList .

E se isso é tudo, por que se preocupar em usar identity function com ternary operator ou usar functor para análise de entrada? Além disso, por que você acha que é uma abordagem adequada aplicar functional programming , que causa efeitos colaterais ( mudando a lista)? Por que todas essas coisas, quando tudo que você precisa é exatamente isso:

const processFooPages = (foo) => foo ? [['foo', foo]] : [];
const processBarPages = (bar) => bar ? bar.map(page => [page.name, page.data]) : [];
const processBazPages = (baz) => baz ? baz.map(page => [page.id, page.content]) : [];

const addPages = (apiData) => {
  const list = new PagesList();
  const pages = [].concat(
    processFooPages(apiData.pages.foo),
    processBarPages(apiData.pages.arrayOfBars),
    processBazPages(apiData.pages.customBazes)
  );
  pages.forEach(([pageName, pageContent]) => list.addPage(pageName, pageContent));

  return list;
}

(um jsfiddle executável aqui )

Como você pode ver, essa abordagem ainda usa functional programming , mas com moderação. Observe também que, como todas as 3 funções de transformação não causam nenhum efeito colateral, elas são fáceis de testar. O código em addPages também é trivial e despretensioso que novatos ou especialistas possam entender com apenas um simples olhar.

Agora, compare este código com o que você criou acima, você vê a diferença? Sem dúvida, as sintaxes functional programming e ES6 são poderosas, mas se você dividir o problema da maneira errada com essas técnicas, você acabará com um código ainda mais confuso.

Se você não se apressar no problema e aplicar as técnicas corretas nos lugares certos, poderá ter o código funcional, embora ainda seja muito organizado e mantido por todos os membros da equipe. Essas características não são mutuamente exclusivas.

O segundo trecho não é mais testável que o primeiro. Seria razoavelmente simples configurar todos os testes necessários para qualquer um dos dois trechos.

A diferença real entre os dois trechos é a compreensão. Eu posso ler o primeiro trecho rapidamente e entender o que está acontecendo. O segundo trecho, não tanto. É muito menos intuitivo, bem como substancialmente mais longo.

Isso facilita a manutenção do primeiro snippet, que é uma qualidade valiosa de código. Eu acho muito pouco valor no segundo trecho.

TD; DR

1. Você pode explicar seu código para o desenvolvedor júnior em 10 minutos ou menos?
2. Daqui a dois meses, você consegue entender seu código?

Análise detalhada

Clareza e legibilidade

O código original é impressionantemente claro e fácil de entender para qualquer nível de programador. Está em um estilo familiar a todos .

A legibilidade é largamente baseada na familiaridade, não em alguma contagem matemática de fichas . IMO, neste estágio, você tem muito ES6 em sua reescrita. Talvez daqui a alguns anos eu mude essa parte da minha resposta. :-) BTW, eu também gosto da resposta @ b0nyb0y como um compromisso razoável e claro.

Testabilidade

if(apiData.pages.foo){
   pagesList.add('foo', apiData.pages.foo){
}

Supondo que o PagesList.add () tenha testes, o que deve acontecer, este é um código completamente simples e não há razão óbvia para que esta seção precise de testes separados especiais.

if (apiData.pages.arrayOfBars){
      let bars = apiData.pages.arrayOfBars;
      bars.forEach((bar) => {
         pagesList.add(bar.name, bar.data);
      })
   }

Mais uma vez, não vejo necessidade imediata de qualquer teste separado especial desta seção. A menos que PagesList.add () tenha problemas incomuns com nulos ou duplicados ou outras entradas.

if (apiData.pages.customBazes) {
      let bazes = apiData.pages.customBazes;
      bazes.forEach((baz) => {
         pagesList.add(customBazParser(baz)); 
      })
   } 

Este código também é muito simples. Assumindo que customBazParser é testado e não retorna muitos resultados "especiais". Então, novamente, a menos que haja situações complicadas com 'PagesList.add (), (o que poderia haver como não estou familiarizado com seu domínio), não vejo por que esta seção precisa de testes especiais.

Em geral, testar toda a função deve funcionar bem.

Aviso Legal : Se houver razões especiais para testar todas as 8 possibilidades das três declarações if() , então sim, divida os testes. Ou, se PagesList.add() for sensível, sim, divida os testes.

Estrutura: Vale a pena dividir em três partes (como o gaulês)

Aqui você tem o melhor argumento. Pessoalmente, não acho que o código original seja "muito longo" (não sou fanático por SRP). Mas, se houvesse mais algumas seções de if (apiData.pages.blah) , o SRP levantaria a cabeça feia e valeria a pena ser dividido. Especialmente se o DRY se aplicar e as funções puderem ser usadas em outros lugares do código.

Minha sugestão

YMMV. Para salvar uma linha de código e alguma lógica, posso combinar o if e o let em uma linha: por exemplo.

let bars = apiData.pages.arrayOfBars || [];
bars.forEach((bar) => {
   pagesList.add(bar.name, bar.data);
})

Isso falhará se apiData.pages.arrayOfBars for um Number ou String, mas também o código original. E para mim é mais claro (e um idioma excessivamente usado).