pagarme / javascript-style-guide

:art: Javascript styleguide followed by us here at Pagar.me
MIT License
50 stars 8 forks source link

No use before define rule. #21

Closed itsdaiego closed 6 years ago

itsdaiego commented 6 years ago

Olá pessoas!

Queria a opinião de vocês sobre uma regra que vem me incomodando um pouco a regra é essa aqui (se o titulo não estiver claro): https://eslint.org/docs/rules/no-use-before-define

Apesar de ver pontos positivos nessas regras tem um ponto em específico que me deixa um tanto "encucado", exemplo:

Eu prefiro isso:


return Promise.resolve()
 .then(doSomething)
 .then(doSomethingElse)

function doSomething () {
 // do something
}

function doSomethingElse () {
 // do something
}

Do que isso:

function doSomething () {
 // do something
}

function doSomethingElse () {
 // do something
}
return Promise.resolve()
 .then(doSomething)
 .then(doSomethingElse) 

Por que?

Eu acho que se você nomear bem as funções, quem ler o primeiro código vai simplesmente ler as 3 primeiras linhas e saber se precisa e onde precisar mudar alguma coisa. No segundo exemplo ele passa primeiro pelas IMPLEMENTAÇÕES das funções e ele não necessariamente precisaria/quer saber como elas foram feitas....

Claro que nesse exemplo isso não faz muita diferença, mas acredito que com lógicas mais complexas isso mais cofunde do que deix "explicito".

O meu desconforto é especialmente a funções e não para variáveis por exemplo...

Queria ouvir a opinião de vocês e saber os pontos a favor dessa regra para que eu possa viver em paz novamente haha

Valeu!!

felquis commented 6 years ago

Sou a favor de user o hoisting das functions pra usar as funções nas promises primeiro.

felquis commented 6 years ago

@drodrigo manda um PR? E o pessoal revisa e da approve

thalesmello commented 6 years ago

Function hoisting é melhor comportamento esquisito do JavaScript. Códigos ficam simplesmente mais elegantes.

MarcoWorms commented 6 years ago

@drodrigo manda PRs!

itsdaiego commented 6 years ago

vou tentar fazer hoje @MarcoWorms tá no meu TO-DO list ;)

mccraveiro commented 6 years ago

Para colocar essa exceção nas regras vote com 👍 neste comentário. Para manter como está, vote com 👎

Let the games begin

wilkmaia commented 6 years ago

Sou contrário à mudança.

return Promise.resolve([1, 2])
  .spread(sum)

function sum (a, b) {
  return a * b
}

Este exemplo denota um caso onde o resultado da função NÃO tem absolutamente nada a ver com seu nome. Sim, é um caso bastante extremista. Mas, muitas vezes, os nomes não são suficientes para indicar tudo o que está acontecendo dentro da função.

Outra questão é que aceitar que o comportamento da função está "perfeito" é um cenário bastante propenso a bugs. "Perfeito" com aspas porque isso é subjetivo ao escopo do que a função diz fazer e ao entendimento subjetivo de cada um.

Semanticamente, não faz sentido usar algo que não está definido, a meu ver.

O único argumento que você levantou foi de que a pessoa tem que ler a definição antes. Ela não tem. Mas ela deveria, independente de onde essa definição está. Há uma sutil diferença aí.

Enfim, são meus 2 centavos sobre o tema.

thalesmello commented 6 years ago

Não existe nenhuma maneira de validar que o nome reflete inteiramente o comportamento da função, verdade. Mas ainda assim, na maioria dos casos, o nome reflete sim a intenção do que a função faz.

O nome também não consegue captar tudo, mas conseguir ler a cadeia de Promises com nomes de ações a serem executadas ajuda a ter um contato inicial com um código desconhecido, dando informações sobre a intenção macro daquele trecho de código. Por sua vez, isso facilita o entendimento da implementação das funções, propriamente dita.

Mesmo que exigíssemos a declaração das função antes de sua utilização, ainda haveria meios de se obter os mesmos resultados:

function main () {
  return Promise.resolve([1, 2]).spread(sum)
}

function sum (a, b) {
  return a + b
}

main()

JavaScript apenas fornece um jeito menos verboso de se escrever a mesma coisa que no código acima,

só que de um jeito menos burocrático. Pessoas de outros backgrounds de linguagens de programação

costumam estranhar esse behavior, mas é muito muito bom, depois que se passa da fase de estranhesa.

Meus outros dois centavos. Acho que vale à pena colocar os nomes antes, para depois entender as definições das funções.

mccraveiro commented 6 years ago

@thalesmello Seu exemplo aí em cima não é válido para o linter hoje. A função sum deveria ser declarada acima da main.

PS: Se vocês quiserem ir ao extremo, da para falar mal do linter com esse exemplo 😆 :

function foo (x) {
  if (x === 0) return 0
  return bar(x)
}

function bar (x) {
  return foo(x - 1)
}

foo(1)
wilkmaia commented 6 years ago

Discordo quando você diz que é menos verbodo, @thalesmello.

Não é menos nem mais. Você escreve exatamente as mesmas informações. O que muda é a ordem com que são escritas. E essa ordem não garante que quem lê vai ler as definições. Apenas facilita isso para quem deseja.

Óbvio que mudar a ordem não impede ninguém de ler as definições antes de ler seu uso. Mas, semanticamente, é o que faz mais sentido, a meu ver. Você não usa algo que não existe.

Sobre nomes de funções indicarem seu comportamento, isso costuma ser válido para funções muito pequenas e com escopo bem definido. O que, nem de longe, é a nossa realidade.

itsdaiego commented 6 years ago

@felquis @MarcoWorms votem ai :tongue: !!!

MarcoWorms commented 6 years ago

@drodrigo pra mim os 2 lados tem argumentos válidos, abstenho meu voto pois acho de boa trabalhar dos 2 jeitos :P

felquis commented 6 years ago

The pool is running https://github.com/pagarme/javascript-style-guide/issues/21#issuecomment-334035502 the PR is opened #23

Tudo o que vocês disseram, me faz achar que é melhor deixar da forma que está hoje, e vamo que vamo!

itsdaiego commented 6 years ago

Fechando esse PR pq a votação está rolando por vários dias e já temos um resultado haha

Valeu pela discussão pessoal =)