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

Implementar função para remover símbolos específicos de uma string de CPF #444

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

Conversation

vmagueta
Copy link

@vmagueta vmagueta commented Oct 24, 2024

Descrição

Implementada a função remove_symbols_cpf, que irá remover os símbolos . e - do CPF fornecido. Em caso de envio de string vazia, ou um inteiro, ou um CPF faltando números ou com números a mais, irá retornar None. Também implementada a Docstring e a doctest, conforme solicitado na issue #437. Adicionado os testes unitários referente a essa função, testando positivamente para remover os symbols, negativo para cpf inválidos ou não formatados.

Checklist de Revisão

  • Eu li o Contributing.md
  • Os testes foram adicionados ou atualizados para refletir as mudanças (se aplicável).
  • Foi adicionada uma entrada no changelog / Meu PR não necessita de uma nova entrada no changelog.
  • A documentação em português foi atualizada ou criada, se necessário.
  • Se feita a documentação, a atualização do arquivo em inglês.
  • Eu documentei as minhas mudanças no código, adicionando docstrings e comentários. Instruções
  • O código segue as diretrizes de estilo e padrões de codificação do projeto.
  • Todos os testes passam. Instruções
  • O Pull Request foi testado localmente. Instruções
  • Não há conflitos de mesclagem.

Issue Relacionada

Closes #437

@vmagueta vmagueta requested review from a team as code owners October 24, 2024 20:20
@vmagueta vmagueta force-pushed the remove_symbols_cpf branch 3 times, most recently from e73d19f to 4e17a25 Compare October 24, 2024 20:57
Copy link

codecov bot commented Oct 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.79%. Comparing base (4af1cdc) to head (831aead).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #444   +/-   ##
=======================================
  Coverage   99.78%   99.79%           
=======================================
  Files          18       18           
  Lines         472      481    +9     
=======================================
+ Hits          471      480    +9     
  Misses          1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vmagueta vmagueta changed the title 437 Issue 437 Oct 24, 2024
brutils/cpf.py Outdated
Comment on lines 275 to 289
cpf_no_symbols = ""
if cpf == "":
return None
elif not isinstance(cpf, str):
return None
else:
for character in cpf:
if character not in ".-":
cpf_no_symbols += character
if len(cpf_no_symbols) < 10:
return None
elif len(cpf_no_symbols) > 11:
return None
else:
return cpf_no_symbols
Copy link

@FabioCruzDev FabioCruzDev Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cpf_no_symbols = ""
if cpf == "":
return None
elif not isinstance(cpf, str):
return None
else:
for character in cpf:
if character not in ".-":
cpf_no_symbols += character
if len(cpf_no_symbols) < 10:
return None
elif len(cpf_no_symbols) > 11:
return None
else:
return cpf_no_symbols
result = None
if isinstance(cpf, str) and cpf != "":
cpf_no_symbols = ''.join([char for char in cpf if char not in ".-"])
if len(cpf_no_symbols) == 11:
result = cpf_no_symbols
return result

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creio q dessa maneira fica mais legivel o codigo, verificamos o tipo do cpf apenas em 1 if, utilizamos um list-comprehension para lidar com o tratamento da string que fica mais enxuto e mais legivel tambem, e deixamos apenas um retorno na função, oq é o ideal.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obrigado pelo retorno, fabio. Já alterei conforme solicitado. Realmente ficou muito melhor mesmo. Muito obrigado!

brutils/cpf.py Outdated
"""

result = None
cpf_no_symbols = ""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pode remover essa linha aqui, nao esta fazendo mais nada.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fabio, qual linha? Desculpe, sou iniciante ainda.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vmagueta claro sem problemas mano, pode remover a linha 276 onde esta sendo declarada a variavel cpf_no_symbols, não precisamos fazer essa atribuição aqui pois estamos declarando ela com o valor na linha 278.

@niltonpimentel02
Copy link

@vmagueta fala meu nobre! uma dica, sempre coloque um título descritivo no teu pr pra aumentar as chances de revisão como "adiciona função para remover símbolos específicos de cpf" ou algo que deixe claro sobre o que é esse novo código.

o @FabioCruzDev já deu umas dicas boas pra melhorar o código mas ainda podemos melhorar um pouquinho mais.. se quiser podemos marcar uma call rapidinha e fazemos juntos.. te mandei uma mensagem no linkedin 😄

@vmagueta
Copy link
Author

@vmagueta fala meu nobre! uma dica, sempre coloque um título descritivo no teu pr pra aumentar as chances de revisão como "adiciona função para remover símbolos específicos de cpf" ou algo que deixe claro sobre o que é esse novo código.

o @FabioCruzDev já deu umas dicas boas pra melhorar o código mas ainda podemos melhorar um pouquinho mais.. se quiser podemos marcar uma call rapidinha e fazemos juntos.. te mandei uma mensagem no linkedin 😄

Poxa, com certeza @niltonpimentel02 , vai ser muito bom ter esta tua ajuda em call, se não for te atrapalhar! Inclusive, já te respondi no Linkedin. Muitíssimo obrigado!

@vmagueta vmagueta changed the title Issue 437 Implementar função para remover símbolos específicos de uma string de CPF - Issue 437 Oct 28, 2024
@vmagueta vmagueta changed the title Implementar função para remover símbolos específicos de uma string de CPF - Issue 437 Implementar função para remover símbolos específicos de uma string de CPF Oct 28, 2024
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.

Implementar função para remover símbolos específicos de uma string de CPF
3 participants