Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Sugestões] Melhorias no suporte para TypeScript + Error handling #224

Open
felinto-dev opened this issue Jul 21, 2021 · 6 comments
Open

Comments

@felinto-dev
Copy link

felinto-dev commented Jul 21, 2021

Infelizmente não tenho conhecimento para fazer esse PR, mas gostaria de dá sugestões que poderiam ser adicionadas no roadmap do projeto baseado no que outras bibliotecas e o mercado como um todo estão adotando:

  1. Melhorar o suporte ao TypeScript na biblioteca (async/await React Native #223 (comment), Import com Typescript #212)

  2. Quando você digita um cep inválido, mesmo com 8 caracteres, a biblioteca retorna um erro dizendo que nenhum provedor encontrou aquele cep. Acho interessante continuar tendo essa opção de retornar um erro, mas também dá a opção para o desenvolvedor de retornar apenas NULL. Acho muito mais legível fazer isso:

const cepData = await cep('58308000', { rejectOnNotFound: false })
if (!cepData) {
    ...envia uma mensagem para o usuário
}

Do que isso:

const cepData = cep('58308000')
    .then((res) => console.log(res))
    .catch(err) => console.log('VOCÊ TRATA O ERRO NO BLOCO CATCH'))

Sobre a convenção de nome "rejectOnNotFound", estou me inspirando no Prisma que é um ORM e utilizam essa mesma convenção:
https://www.prisma.io/docs/reference/api-reference/prisma-client-reference#rejectonnotfound

Validação do CEP:
Ainda sobre error handling, seria legal disparar um erro caso o CEP não tenha 8 caracteres (não cheguei a ver se a biblioteca já faz isso). Poderia usar essa diretiva "rejectOnNotFound" também. Ficaria assim:

const cepData = await cep('58308000', { rejectOnNotFound:  {
    cepNotFound: false,
    cepInvalid: true,
} })

O cep-promise não traz uma função do tipo "cep('58308000').isValid()" e realmente compreendo que talvez os criadores da biblioteca acreditem que isso foge do escopo do projeto. Na minha opinião não foge do escopo, tendo em vista que quando é requisitado dados sobre um cep, usando a biblioteca cep-promise, é esperado que o cep seja válido (8 caracteres, somente números), ou você está apenas fazendo requisições inúteis e gastando banda de 3 - 4 servidores de API que de boa-fé, disponibilizam seus serviços para todos gratuitamente.

Dá a opção de escolher ao desenvolvedor se deveria ser lançado ou não uma exceção quando o cep fosse inválido, seria interessante.

Exemplo: Se eu já fiz a validação do cep usando outra biblioteca, mas mesmo assim o cep chega mal formatado no cep promise, eu como desenvolvedor vou querer que isso sempre gere uma exceção, pois é um comportamento inesperado da minha aplicação.

@fontebasso
Copy link
Contributor

fontebasso commented Jul 23, 2021

Maravilha @felinto-dev eu tenho um carinho super especial por essa lib e adorei suas sugestões!

Alguns pontos para discutirmos:

  • Sinto que a responsabilidade de validação de dados não é mesmo da biblioteca, ela faz as chamadas http e pronto, o dev validando antes, sob a perspectiva que ele julgar melhor nos coloca em uma postura defendida por muitos que é: "faça uma coisa e a faça bem feito". Contudo, seu argumento de evitar requisições que sabemos que serão inúteis foi matador e eu cedo nessa, estou contigo e deveriamos fazer as validações por aqui, mesmo que redundantes. Ainda com tudo isso, gostaria de ouvir mais opiniões;

  • Sobre o rejectOnNotFound, podemos dizer que retornar null ou exceção é até gosto e nada melhor que ter opções, mas já que estamos falando do que pode ser, adoraria irmos na onda do TypeScript e ter exceções diferentes como por exemplo NotFound e ConnectionError para casos de cpf não encontrado e falha de rede em todos os provedores (seria possível? hahaha);

@felinto-dev
Copy link
Author

É verdade @fontebasso. Eu cheguei a comentar justamente isso. Que talvez algumas pessoas acreditem que isso foge do escopo do projeto, mas além do ponto que citei que você está gastando banda e processamento que serviços que de boa fé disponibilizam os dados gratuitamente, não se preocupar com isso não envia uma mensagem muito boa para o mercado.

Para questionamento, será que faz sentido verificar os dados de um cep sem verificar se ele é válido? O desenvolvedor, na maioria dos casos, não vai validar o cep? Pensando isso, acho que não dá para desassociar o ato de obter dados de um cep e validar o mesmo. Sem contar, que verificar se um cep tem 8 dígitos e somente números, é algo facilmente feito por uma regex de 1 linha só. Formatar, se for o caso também. Então não vejo como isso pode atrapalhar o desenvolvimento do projeto, tendo em vista que depois de feito essas funções, não vejo espaço para bugs ou melhorias no futuro.

Em relação ao error handling, um dos principais benefícios de abordar a sintaxe que sugeri, é um código mais enxuto e utilizando um bloco de IF/ELSE em vez de TRY/CATCH ou THEN/CATCH.

@fontebasso
Copy link
Contributor

fontebasso commented Jul 23, 2021

Sim, quanto ao a validação é como citei, estou contigo.

Agora não posso concordar que IF/ELSE é mais enxuto que TRY/CATCH, sou adepto a ideia de evitar usar IF/ELSE, aqui é uma conversa tênue entre melhores práticas e/ou gosto pessoal. Para não incitarmos uma discussão do que é melhor (tipo time de futebol), nada como poder configurar a lib como você mesmo sugeriu, deixa os devs escolherem o que for melhor para seu caso de uso.

@RodriAndreotti
Copy link

Bom, não sou contribuidor oficial do projeto, mas, como dev me sinto no dever de dar meu pitaco (mesmo que irrelevante... rs)
Eu acho que a validação nunca deve ficar a cargo somente do cliente, pois não sabemos se será bem feita ou não, acho sim interessante ter uma segunda camada de validação e evitar os requests desnecessários ;-)

Sobre o IF / ELSE e TRY/CATCH, acho que vou entrar numa enrascada, mas vamos lá... rs
Concordo que em certos casos é uma questão de preferência, mas vale lembrar que cada qual tem uma função bem delimitada, a base de toda linguagem de programação é:

  • Condicionais
  • Loops e
  • Variáveis

O TRY/CATCH só deve ser utilizado em tratamento de exceções, ele foi criado para isso, então se o sistema puder lançar uma exceção para tratar seria excelente tbm, afinal um request errado ou que não retorna o esperado é exatamente isso, uma exceção.
Evitar utilizar IF/ELSE neste caso faz sentido, mas tem N outros casos em que m IF/ELSE é insubstituível, até porque ele é uma das bases de qualquer linguagem de programação.

Enfim, meus 2 cents, se não for relevante para o assunto, fiquem a vontade para ignorar solenemte XD

@felinto-dev
Copy link
Author

felinto-dev commented Jul 30, 2021

@RodriAndreotti acho que o @fontebasso já abordou esse ponto e ele explicou isso muito bem:

Agora não posso concordar que IF/ELSE é mais enxuto que TRY/CATCH, sou adepto a ideia de evitar usar IF/ELSE, aqui é uma conversa tênue entre melhores práticas e/ou gosto pessoal. Para não incitarmos uma discussão do que é melhor (tipo time de futebol), nada como poder configurar a lib como você mesmo sugeriu, deixa os devs escolherem o que for melhor para seu caso de uso.

Já abordamos esse ponto aqui também:

Eu acho que a validação nunca deve ficar a cargo somente do cliente, pois não sabemos se será bem feita ou não, acho sim interessante ter uma segunda camada de validação e evitar os requests desnecessários ;-)

Com 2 argumentos:
O primeiro argumento é que um simples código, talvez com 1 linha que verifique se o CEP tem somente números e 8 dígitos evitaria requisições desnecessárias à API. E o segundo argumento é que na minha opinião se você for requerer dados de um CEP você NECESSARIAMENTE vai querer verificar se o CEP é válido antes de enviar a requisição para API. Por isso é algo que acredito que seja totalmente relacionado ao módulo:

O cep-promise não traz uma função do tipo "cep('58308000').isValid()" e realmente compreendo que talvez os criadores da biblioteca acreditem que isso foge do escopo do projeto. Na minha opinião não foge do escopo, tendo em vista que quando é requisitado dados sobre um cep, usando a biblioteca cep-promise, é esperado que o cep seja válido (8 caracteres, somente números), ou você está apenas fazendo requisições inúteis e gastando banda de 3 - 4 servidores de API que de boa-fé, disponibilizam seus serviços para todos gratuitamente.

Ademais, se você tiver algum ponto a favor ou contra qualquer uma das ideias apresentadas acima, que foi passado batido, por favor, não deixe de avisar para que possamos melhorar a biblioteca para todos.

@RodriAndreotti
Copy link

Com 2 argumentos:
O primeiro argumento é que um simples código, talvez com 1 linha que verifique se o CEP tem somente números e 8 dígitos evitaria requisições desnecessárias à API. E o segundo argumento é que na minha opinião se você for requerer dados de um CEP você NECESSARIAMENTE vai querer verificar se o CEP é válido antes de enviar a requisição para API. Por isso é algo que acredito que seja totalmente relacionado ao módulo:

Então, mas eu realmente concordo com isso, a validação nunca deve ficar a cargo somente do lado do qual não se tem controle, acho importantíssimo que se valide a informação do lado da lib, primeiro pelos motivos que já foram apontados por você e segundo, porque não necessariamente pode vir um cep inválido, mas alguma coisa a mais (alguma tentativa de XSS, por exemplo), que por ventura a lib encaminharias para as API's

Sobre a questão do try/catch, só quis discorrrer um pouco mais, porque acho importante que o código diga o que ele está fazendo, pois quem fará manutenção pode não ser a mesma pessoa que desenvolveu, sendo assim, acho muito importante usar cada coisa para sua devida função, IF/ELSE tem uma funcionalidade bem definida, assim como TRY/CATCH, mas é só isso mesmo.

E, realmente, o intuito é que a LIB fique cada vez melhor, como falei, não sou contribuidor dessa lib, mas hoje sou contribuidor do BrasilAPI, que a utiliza, temos uma grande escassez de boas libs nacionais.

Abraços ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants