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

Adicionando ddd no retorno da busca pelo cep referente a issue aberta #416 #455

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Carlosvpm
Copy link

A alteração consiste na adição de uma função que busca o DDD da região pelo nome da cidade retornado pelo end-point ja existente do cep, utilizei os recursos ja existentes no módulo de ddd.

@vercel
Copy link

vercel bot commented Jan 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
brasilapi ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 6, 2023 4:18pm

@sonarcloud
Copy link

sonarcloud bot commented Jan 11, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@LorhanSohaky
Copy link
Member

@Carlosvpm , será que vale a pena criar um endpoint que seja /v1/cep-e-ddd ou /v3/cep?

Só para explicar em mais detalhes... O endpoint v2 já é mais demorado que o v1 poq ele precisa bater em outro fornecedor de dados para pegar a geolocalização aproximada. E com a adição do dado do ddd isso ficará mais demorado.

Uma outra opção que não degrada no quesito tempo de execução e acho bem interessante é adicionar suporte a HATEOAS (#329). Nessa situação sugeriria criar um /v3/cep com esse link para consultar o ddd sem afetar a performance da rota

@Carlosvpm
Copy link
Author

Carlosvpm commented Jan 23, 2023

@Carlosvpm , será que vale a pena criar um endpoint que seja /v1/cep-e-ddd ou /v3/cep?

Só para explicar em mais detalhes... O endpoint v2 já é mais demorado que o v1 poq ele precisa bater em outro fornecedor de dados para pegar a geolocalização aproximada. E com a adição do dado do ddd isso ficará mais demorado.

Uma outra opção que não degrada no quesito tempo de execução e acho bem interessante é adicionar suporte a HATEOAS (#329). Nessa situação sugeriria criar um /v3/cep com esse link para consultar o ddd sem afetar a performance da rota

@LorhanSohaky, Realmente, não me atentei para a performance do recurso, fiz a medição das implementações e a diferença foi de quase 4x o tempo na V2 implementada, em contrapartida utilizando o padrão HATEOAS o tempo de execução diminui significativamente.
Desse modo, manterei com o padrão HATEOAS. Porém, creio que não seja a melhor escolha pra quem esteja consumindo, porque irá fazer mais requisições que o necessário. talvez se conseguirmos fazer algo como o graphQL faz, seja a melhor opção neste caso. Precisarei estudar outras opções. Agradeço pelo feedback!! TMJ!
image

@LorhanSohaky
Copy link
Member

Uma coisa que me ocorreu agora....

Se for muito importante vir o dado de DDD na resposta da API, você pode colocar uma parâmetro para incluir ou não esse dado na resposta /v3/cep/12345?ddd=true e na resposta já vem o DDD. E para melhorar a performance você pode tentar paralelizar o request de geolocalização com o ddd. Além disso, seria interessante se fosse possível algum tipo de "memo" para evitar de fazer requisição ao provedor do DDD, isso tornaria a API rápida

@LorhanSohaky
Copy link
Member

@lucianopf , @murilohns , o que acham?

@lucianopf
Copy link
Member

Uma coisa que me ocorreu agora....

Se for muito importante vir o dado de DDD na resposta da API, você pode colocar uma parâmetro para incluir ou não esse dado na resposta /v3/cep/12345?ddd=true e na resposta já vem o DDD. E para melhorar a performance você pode tentar paralelizar o request de geolocalização com o ddd. Além disso, seria interessante se fosse possível algum tipo de "memo" para evitar de fazer requisição ao provedor do DDD, isso tornaria a API rápida

Curti as propostas hein!
Faria inclusive a busca de geolocalização ser parametrizável tbm 🤔

Talvez não fosse necessário nem criar uma v3 se a gnt manter o padrão da v2 como geo=true&ddd=false pq não quebraria a interface de quem já está consumindo 👀

@sonarcloud
Copy link

sonarcloud bot commented Apr 24, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@lucianopf lucianopf force-pushed the main branch 5 times, most recently from d29a1aa to bf3a71b Compare May 8, 2023 15:58
@sonarcloud
Copy link

sonarcloud bot commented Aug 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

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

Successfully merging this pull request may close these issues.

None yet

3 participants