quero-edu / guidelines

📘 The definitive Quero Education code-style guide.
MIT License
22 stars 2 forks source link

Discussão sobre a regra class-methods-use-this #57

Closed rickfreitas closed 3 years ago

rickfreitas commented 3 years ago

No nosso time ja levantamos algumas vezes a discussão sobre manter ou não essa regra, gostaria de trazer para cá para podermos apreciar mais opiniões 😅 https://eslint.org/docs/rules/class-methods-use-this

Ela ajuda bastante a separar o que deveria ou não estar na classe X, ou até se deveria ser uma classe, mas em alguns casos a classe tem funções que não utilizam o this, mas se remove-las parece que a classe fica um tanto quanto incompleta.

vitebo commented 3 years ago

Na minha opinião eu queimaria esse cara fácil, ele só te força a deixar o código descentralizado. Acontece varias vezes isolar um utilitário ou qualquer coisa do tipo em um método apenas para deixar o código mais legível (e até reaproveitar em outros métodos), e na maior parte dos casos (dos meus pelo menos) é algum tipo de utilitário que é muito especifico para aquele caso de uso, não faz sentido jogar esse cara pra um nível "mais global" no projeto.

E a partir do momento que essa regra ta desativada, se surgir alguma sugestão em algum PR dizendo para mover trecho X do código para um outro lugar porque poderia ser reutilizável e tals, na minha visão isso tbm não é um questionamento de lint (que é o que a gente tenta evitar no PRs centralizando as regras aqui), é muito mais um questionamento de arquitetura de código, e super plausível em um CR.

Faz muito tempo que não trabalho com classes (ultimamente to trabalhando só com componentes) então eu nem sei se essa regra ta habilitada ou não hoje mas eu ja sofri muito com ela la no Pandora, então deixo os meus 10 centavos sobre o assunto aqui, espero que ajude

toledompm commented 3 years ago

Eu gosto dessa regra :peepo-shy:. Principalmente por esses 2 motivos:

Só que concordo com o que o @vitebo comentou, o "problema" que essa regra tenta solucionar, é mais de arquitetura do que de style, por esse motivo concordaria em :scissors:

mateusppereira commented 3 years ago

Já fui muito contra remover essa regra... mas depois de ler algumas coisas nesse link que o @toledompm mandou e diante do cenário que não é a primeira vez que estamos discutindo sobre ela, eu estou muito convencido que é dor de cabeça demais pra pouco valor ❤️

heitorrbarros commented 3 years ago

Acho uma boa deixar o lint cuidar disso. Penso: "Se tá resolvendo um problema de contexto(arquitetura) de graça... Aceito com muito amor."

Se o projeto tem uma class e dentro dela o enforcement do this não é realizado, desconfio que alguém que saiba menos de OO e padrões apenas ignore e não faça this.sayHi(). Isso seria ruim, tenho que pensar mais para entender de onde o método vem...

Javascript é uma lang irada. Logo, é muito easy mover um método que não usa nada da classe para fora dela ou até mesmo exportar como uma função, se o static não fizer sentindo, fica até mais fácil de testar.

No fim, deixar essa regra seria delicinha pra mim.

toledompm commented 3 years ago

Decidimos manter a regra pois a maioria achou que faz sentido.