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

Adicionar função format #148

Open
dmvvilela opened this issue Nov 26, 2019 · 11 comments
Open

Adicionar função format #148

dmvvilela opened this issue Nov 26, 2019 · 11 comments
Assignees

Comments

@dmvvilela
Copy link

Exemplo: https://www.npmjs.com/package/cpf

Para fazer por exemplo cep.format(XXXXXXXX) e sair XX.XXX-XXX

Ia ser legal ter isso! :)

@lucianopf
Copy link
Member

lucianopf commented Nov 27, 2019

@dmvvilela a ideia é boa sim, apesar de ser mega simples de resolver isso no próprio client, só não sei mensurar o tamanho do benefício em relação a quebra de interface que isso vai causar na lib 🤔

Inicialmente pensando só consigo imaginar uma nova funcionalidade dessa surgindo se a gnt modificar o que exportamos do módulo pra ao invés de ser uma função ser um objeto contendo dois novos métodos, tipo:

{
  find: () => { ... },
  format: () => { ... },
}

Talvez assim a gnt consiga inclusive reutilizar as funções internas e exportar elas pra que clientes implementem novas soluções 🤔

@filipedeschamps , preciso da sua opinião aqui tendo em vista que isso implicaria em breaking changes

Função simples que já resolveria esse seu probleminha:

const formatAsCep = value => String(value).replace(/^(\d{5})(\d{0,3})(.*)/, "$1-$2")

@dmvvilela
Copy link
Author

É coisa pequena mesmo, só uma idéia pra deixar a lib mais fera!

Seguindo a lib que coloquei ali, ela facilita muito por exemplo, já faz o strip do termo, retirando .,/ etc e faz a pesquisa independente da máscara.

Isso permite a gente no client utilizar qualquer máscara e pesquisar com facilidade sem ter que fazer o strip na mão.

É coisa bem pequena, mas que ajuda :)

@filipedeschamps
Copy link
Member

Fala pessoal! Show essa idéia! Minha sugestão seria retornar isso no response para evitar breaking change, algo como:

cep('05010000')
  .then(console.log)

  // {
  //   "cep":  "05010000",
  //   "cepFormatted":  "05.010-000",
  //   "state":  "SP",
  //   "city":  "São Paulo",
  //   "street":  "Rua Caiubí",
  //   "neighborhood":  "Perdizes",
  // }

Um dos downsides que consigo pensar é se a pessoa só precisar formatar um cep sem querer fazer a consulta (apesar de que esse não é o use case da biblioteca).

O que acham?

@vagnercardosoweb
Copy link
Contributor

vagnercardosoweb commented Feb 11, 2020

Por padrão o viacep já vem com ele formatado e na aplicação é removido o - (hífen), acho que seria interessante a quem estiver usando a lib decidir formatar ou não, até porque a formatação pode ser diferente em alguns casos.

@dansoliveira
Copy link

Show!! Seguindo a linha do @vagnercardosoweb, seria interessante um campo de options. Exemplo:

cep('05010000', { format: true }).then(console.log)

E, para evitar breaking changes, poderíamos definir um valor default para este objeto de forma que continue com o funcionamento padrão.

Vejo que seria um bem mais trabalhoso de realizar a inclusão desse objeto nos services (até mesmo por conta das promises), principalmente se comparado com a solução proposta pelo Teló @filipedeschamps.

Inclusive, entre as ideias, entendo que teríamos de escolher entre:

  • ter ambos os valores (com/sem máscara) e evitar do usuário passar mais parâmetros;

ou

  • permitir a customização do valor retornado atualmente de acordo com a necessidade, evitando recuperar múltiplos parâmetros para uma mesma finalidade.

Realmente não sei até que ponto seria bom ambos os casos hahaha

PS: Estou muito animado com o rumo deste projeto. Espero contribuir e aprender muito. Está sendo a minha primeira participação num projeto Open Source 😍

@SkyaTura
Copy link

Eu gostaria de adicionar a discussão que existem duas possibilidades muito comuns de formatação CEP:

12.345-678 e 12345-678

@filipedeschamps
Copy link
Member

Show! Decisão boa! Eu vejo a situação dessa forma (que é um cálculo de tradeoff)

  • Incrementar a interface: isso é algo negativo, é algo a mais para ler na documentação, por exemplo.
  • Receber o cep formatado: isso é algo positivo para quem precisa.

Para o tradeoff acima, acho que o saldo é negativo, pois eu vejo o seguinte cenário:

  • Não incrementar a interface: isso é algo neutro.
  • Receber o cep formatado: isso é algo positivo para quem precisa e neutro para quem não precisa.

Como a máscara pode ser aplicada localmente, não é necessário trafegar isso pela network inclusive, então o custo técnico é muito baixo.

Então ela, sem complicar a interface, pode retornar por padrão:

cep('05010000')
  .then(console.log)

  // {
  //   "cep":  "05010000",
  //   "cepMask1":  "05010-000",
  //   "cepMask2":  "05.010-000",
  //   "state":  "SP",
  //   "city":  "São Paulo",
  //   "street":  "Rua Caiubí",
  //   "neighborhood":  "Perdizes",
  // }

Como o @SkyaTura falou, essas são as possibilidades mais comuns e vejo que se a pessoa cair em um outro formato, ela deve estar fazendo uma coisa muito específica a aplicação dela e essa especificidade deveria ficar apenas na aplicação dela. Do contrário, não vale a pena pagar o custo técnico e de interface+documentação para atender a esse caso específico.

O que acham?

Fora isso, eu estou com um dilema sobre o BrasilAPI porque a idéia é usar o BrasilAPI como provider, mas ele utiliza internamente o cep-promise, ou seja, vai ficar em um loop. Uma das formas que vejo de evitar isso é, pela interface ou de alguma outra forma, definir quais providers ele quer consultar. Se vocês conseguirem pensar em formas de não alterar a interface e evitar esse loop, topam criar uma outra issue para discutir isso?

Abração turma!!! 👍

@SkyaTura
Copy link

Eu também acho essa solução mais interessante, @filipedeschamps , voto nela!

Sobre o seu dilema, eu adoraria participar dessa discussão também.

Na minha opinião, utilizar o cep-promise internamente na BrasilAPI é mais vantajoso do que utilizar a BrasilAPI como provider, pois uma das melhores características desse projeto é que ele busca em diversos fornecedores para garantir uma resposta, ao passo que se a BrasilAPI for utilizada como provedor, nós ficamos dependentes da disponibilidade dela.

@lucianopf
Copy link
Member

Vai me bater se eu propor uma nova função no protótipo do cep que faz o setup dos providers usados? 😂

const cep = require('cep').config({
  providers: ['ViaCep', 'Correios'],
  masked: true,
})

É meio feio, mas não altera a interface do que já existe em nada 😬

@filipedeschamps
Copy link
Member

Show @SkyaTura a idéia não é fazer o cep-promise ter o BrasilAPI como único provedor, e sim adicionar o BrasilAPI como mais um provider do cep-promise. Só que como de fato o BrasilAPI vai utilizar o cep-promise para o endpoint do cep, no fundo ele vai também fazer uma chamada para si de volta e vai endoidar num loop 😂

E @lucianopf adorei a sugestão!!!!! Realmente gostei! Eu tava com a cabeça presa na instância e não na factory! Eu só retornaria a máscara por default, mas a parte de configurar ali os providers matou a pau!!!!!

@lucianopf
Copy link
Member

Na verdade a alteração da forma que comentei fica bem feia, acho melhor a sugestão do @dansoliveira mesmo 😬

Confesso que já coloquei o pé na água e ficou elegante 😬

cep(
  29090010,
  {
     providers: ['viacep']
  }
)
.then(console.log).catch(console.log)

Sendo que o segundo parâmetro é totalmente opcional 🤔 😬

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

No branches or pull requests

6 participants