Eu sou muito inteligente para ser legível pelos desenvolvedores Jr.? Muita programação funcional no meu JS? [fechadas]

Sou desenvolvedor sênior de front-end, codificando em Babel ES6. Parte de nosso aplicativo faz uma chamada de API e, com base no modelo de dados que recebemos da chamada de API, é necessário preencher determinados formulários.

Esses formulários são armazenados em uma lista duplamente vinculada (se o back-end indicar que alguns dos dados são inválidos, podemos retornar rapidamente ao usuário a página que eles estragaram e depois recuperá-los no destino, simplesmente modificando o Lista.)

De qualquer forma, há várias funções usadas para adicionar páginas, e estou me perguntando se estou sendo muito inteligente. Esta é apenas uma visão geral básica - o algoritmo real é muito mais complexo, com toneladas de diferentes páginas e tipos de página, mas isso lhe dará um exemplo.

Acho que é assim que um programador iniciante pode lidar com isso.

export const addPages = (apiData) => {
   let pagesList = new PagesList(); 

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

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

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

   return pagesList;
}

Agora, para ser mais testável, eu peguei todas essas instruções if e as fiz separadas, funções independentes e depois mapeei sobre elas.

Agora, testável é uma coisa, mas também é legível, e me pergunto se estou tornando as coisas menos legíveis aqui.

// file: '../util/functor.js'

export const Identity = (x) => ({
  value: x,
  map: (f) => Identity(f(x)),
})

// file 'addPages.js' 

import { Identity } from '../util/functor'; 

export const parseFoo = (data) => (list) => {
   list.add('foo', data); 
}

export const parseBar = (data) => (list) => {
   data.forEach((bar) => {
     list.add(bar.name, bar.data)
   }); 
   return list; 
} 

export const parseBaz = (data) => (list) => {
   data.forEach((baz) => {
      list.add(customBazParser(baz)); 
   })
   return list;
}


export const addPages = (apiData) => {
   let pagesList = new PagesList(); 
   let { foo, arrayOfBars: bars, customBazes: bazes } = apiData.pages; 

   let pages = Identity(pagesList); 

   return pages.map(foo ? parseFoo(foo) : x => x)
               .map(bars ? parseBar(bars) : x => x)
               .map(bazes ? parseBaz(bazes) : x => x)
               .value

}

Aqui está a minha preocupação. Para mim, o fundo é mais organizado. O código em si é dividido em partes menores que podem ser testadas isoladamente. MAS, estou pensando: se tivesse que ler isso como desenvolvedor júnior, não acostumado a conceitos como o uso de functores de identidade, currying ou declarações ternárias, seria capaz de entender o que a última solução está fazendo? É melhor fazer as coisas da maneira "errada, mais fácil" às vezes?

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

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

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-o para passar uma lista mutável por 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 - apenas analisando esse item, se existir - ainda está lá explicitamente no seu código, mas agora temos que pensar ao virar da esquina. Por exemplo, é um tanto ó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)?). Não vejo nenhum benefício nessa 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 é 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 podemos adicionar explicitamente às páginas, isso pode ser melhor:

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

Benefício adicional: a lógica sobre o que fazer quando o item está vazio agora foi movida para as funções de análise individuais. Se isso não for apropriado, você ainda poderá introduzir condicionais. A mutabilidade da pageslista agora é reunida em uma única função, em vez de espalhá-la por 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 Monad.

Então, sim, seu código era muito inteligente. Aplique sua inteligência para não escrever um código inteligente, mas para encontrar maneiras inteligentes de evitar a necessidade de uma inteligência flagrante. Os melhores designs não parecem sofisticados, mas parecem óbvios para quem os vê. E existem boas abstrações para simplificar a programação, não para adicionar camadas extras que tenho que desembaraçar em minha mente (aqui, descobrindo que o functor é equivalente a uma variável e pode ser efetivamente elidido).

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

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

O benefício suposto, que você pode testar os blocos individualmente, pode ser alcançado de uma maneira mais simples, apenas dividindo as funções individuais. E no segundo exemplo, você junta as iterações separadas, para obter menos isolamento.

Quaisquer que sejam seus sentimentos sobre o estilo funcional em geral, este é claramente um exemplo em que torna o código mais complexo.

Acho um pouco de sinal de alerta ao associar código simples e direto a "desenvolvedores iniciantes". Essa é uma mentalidade perigosa. Na minha experiência, é o contrário: desenvolvedores iniciantes são propensos a códigos excessivamente complexos e inteligentes, porque exige 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 iniciante pode não entender. Em vez disso, trata-se de escrever código mais complexo ou complicado do que o necessário . Isso torna o código mais difícil de seguir para todos , iniciantes e especialistas, e provavelmente também para você, alguns meses depois.

Esta resposta minha chega um pouco tarde, mas eu ainda quero entrar em cena. Só porque você está usando as mais recentes técnicas de ES6 ou o paradigma de programação mais popular não significa necessariamente que seu código está mais correto ou o código de junior 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 incluir as mais recentes técnicas de programação em todos os problemas, sempre terá uma solução com excesso de engenharia.

Dê um passo atrás e tente verbalizar o problema que você está tentando resolver por um segundo. Em essência, você quer apenas uma função addPagespara transformar diferentes partes de apiDataem um conjunto de pares de valores-chave e depois adicionar todos eles PagesList.

E se isso é tudo que existe para ela, por que se preocupar usando identity functioncom ternary operator, ou utilizando functorpara entrada de análise? Além disso, por que você acha que é uma abordagem adequada a aplicar functional programmingque causa efeitos colaterais (alterando a lista)? Por que todas essas coisas, quando tudo que você precisa é apenas 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 as três funções de transformação não causam efeitos colaterais, elas são fáceis de testar. O código addPagestambém é trivial e despretensioso que iniciantes ou especialistas podem entender com apenas um simples olhar.

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

Se você não se apressar no problema e aplicar as técnicas certas nos lugares certos, poderá ter o código funcional por natureza, enquanto ainda é muito organizado e sustentável 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 é compreensibilidade. Consigo ler o primeiro trecho rapidamente e entender o que está acontecendo. O segundo trecho, nem tanto. É muito menos intuitivo e substancialmente mais longo.

Isso facilita a manutenção do primeiro trecho, que é uma qualidade valiosa do código. 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. É um estilo familiar para todos .

A legibilidade é amplamente baseada na familiaridade, não em uma contagem matemática de tokens . Na IMO, nesta fase, você tem muito ES6 em sua reescrita. Talvez daqui a alguns anos eu mude essa parte da minha resposta. :-) Aliás, 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 PagesList.add () possua testes, o que deveria ser, esse é um código completamente direto e não há motivo óbvio 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);
      })
   }

Novamente, não vejo necessidade imediata de nenhum teste especial e separado desta seção. A menos que o PagesList.add () tenha problemas incomuns com nulos, duplicatas 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 direto. Supondo que customBazParserseja testado e não retorne muitos resultados "especiais". Então, novamente, a menos que haja situações complicadas com o `PagesList.add (), (que pode ser que eu não esteja familiarizado com o 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.

Isenção de responsabilidade : Se houver razões especiais para testar todas as 8 possibilidades das três if()instruções, 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 Gália)

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

Minha sugestão

YMMV. Para salvar uma linha de código e alguma lógica, eu posso combinar o if e deixar 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 o código original também será. E para mim é mais claro (e um idioma muito usado).