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

implement new types of key #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NathanAllmeida
Copy link

No description provided.

@sourcery-ai
Copy link

sourcery-ai bot commented Jun 10, 2022

Sourcery Code Quality Report

❌  Merging this PR will decrease code quality in the affected files by 12.61%.

Quality metrics Before After Change
Complexity 2.25 ⭐ 5.32 ⭐ 3.07 👎
Method Length 27.57 ⭐ 39.62 ⭐ 12.05 👎
Working memory 4.70 ⭐ 7.00 🙂 2.30 👎
Quality 88.86% 76.25% -12.61% 👎
Other metrics Before After Change
Lines 73 139 66
Changed files Quality Before Quality After Quality Change
pixqrcode/service/generate_code.py 89.44% ⭐ 82.28% ⭐ -7.16% 👎
pixqrcode/service/validate_pix.py 88.70% ⭐ 75.15% ⭐ -13.55% 👎

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation
pixqrcode/service/generate_code.py GenerateCode.generate 0 ⭐ 91 🙂 21 ⛔ 57.40% 🙂 Extract out complex expressions
pixqrcode/service/validate_pix.py ValidatePix.detect_type_key 12 🙂 92 🙂 10 😞 58.61% 🙂 Extract out complex expressions
pixqrcode/service/validate_pix.py ValidatePix.validateCPF 4 ⭐ 129 😞 7 🙂 67.92% 🙂 Try splitting into smaller methods

Legend and Explanation

The emojis denote the absolute quality of the code:

  • ⭐ excellent
  • 🙂 good
  • 😞 poor
  • ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.


Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

@NathanAllmeida
Copy link
Author

NathanAllmeida commented Jun 10, 2022

Olá
Vi seu repositório e vi que aceitava apenas chave telefone, fiz umas mudanças para aceitar outras. Sou iniciante em python então aceito sugestões também

Copy link
Owner

@Mostela Mostela left a comment

Choose a reason for hiding this comment

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

Tem algumas correções simples como nome de variável que não segue o padrão do código, alguns blocos de código um pouco confusos e algumas coisas ligadas a padrão de projetos que pode ser melhorado.

return f"00020126360014{self.merchant.globally_unique_identifier}0114{pix.mobile}520400005303986540" \
valPix = ValidatePix(pix)
detect_type = valPix.detect_type_key
code = detect_type()
Copy link
Owner

Choose a reason for hiding this comment

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

Tu pode instanciar a classe direto e já puxar o método não precisaria passar para dentro de outra variável.
Mesma coisa o code = detect_type() tu pode passar o método já direto dentro da string, ficaria igualmente legível.

@@ -11,7 +12,11 @@ def left_zero(self, text: str):
return str(len(text)).zfill(2)

def generate(self, pix: Pix):
return f"00020126360014{self.merchant.globally_unique_identifier}0114{pix.mobile}520400005303986540" \
valPix = ValidatePix(pix)
Copy link
Owner

Choose a reason for hiding this comment

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

Aqui seguir o padrão de ter lowercase (snake_case) para as variáveis dentro de funções

if len(numbers) != 11 or len(set(numbers)) == 1:
return False

sum_of_products = sum(a*b for a, b in zip(numbers[0:9], range(10, 1, -1)))
Copy link
Owner

Choose a reason for hiding this comment

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

A logica de validação de CPF ficou um pouco confusa sobre qual digito é. Coloque um comentário para ajudar a ler no futuro sobre qual é qual mais precisamente, vai ajudar bastante quando tu tem blocos de códigos parecidos

def mobile(self):
if not self.pix.mobile:
raise PixError("telefone nao informado")
def validateCPF(self):
Copy link
Owner

Choose a reason for hiding this comment

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

o padrão para o nome dos métodos também segue como lowercase (snake_case). Mesmo CPF sendo "nome" o aconselhável é seguir como os demais métodos.

"""
def detect_type_key(self):
key = self.pix.mobile
regex = re.search(r'^[a-zA-Z0-9._-]+@[a-zA-Z0-9]+\.[a-zA-Z\.a-zA-Z]{1,10}$', key)
Copy link
Owner

Choose a reason for hiding this comment

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

o nome da variável pode deixar confuso durante o uso ao longo do código, talvez ser mais especifico regex_email = re....

# 0114 - Telefone

code = ""
if regex is None:
Copy link
Owner

Choose a reason for hiding this comment

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

No fluxo de validação da chave ele acaba realizando uma dupla função, Descobrir qual tipo de chave é se é email, cpf, cnpj ou telefone e validar a chave, isso torna o fluxo mais difícil de entender e tb com if um dentro do outro como é esse o caso aqui.

Ai o que é interessante de se fazer, ter fluxos distintos para isso. Em um você Descobrir qual tipo de chave é e o segundo validar a chave com base no tipo se houver um match.

Podendo assim ter vários métodos um para cada tipo de chave, e ai chamar um método que identifica o tipo de chave no validade (linha 14) e chama o método corresponde para aquele tipo de chave.

else:
self.pix.mobile = f"+{self.pix.mobile}"
raise Exception("A chave informada não foi identificada")
Copy link
Owner

Choose a reason for hiding this comment

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

Voltar o Exception aqui vai disparar um erro que não está padronizado para tratamento. Utilizar a classe PixError, ai quando houver um erro que precise ser reportado tu não corre o risco de parar o código simulando uma Exception genérica.

@@ -11,7 +12,11 @@ def left_zero(self, text: str):
return str(len(text)).zfill(2)

def generate(self, pix: Pix):
return f"00020126360014{self.merchant.globally_unique_identifier}0114{pix.mobile}520400005303986540" \
valPix = ValidatePix(pix)
Copy link
Owner

Choose a reason for hiding this comment

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

O ValidatePix é chamado no init.py dentro do metodo is_valid()
O code tu pode passar ele utilizando a DTO Pix o parametro que a classe recebe

def mobile(self):
if not self.pix.mobile:
raise PixError("Chave nao informado")
code = self.detect_type_key()
Copy link
Owner

Choose a reason for hiding this comment

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

Em um debug do código, o método de detect_type_key executou duas vezes. Eu deduzo que isso não seja o comportamento espero

if not self.pix.mobile:
raise PixError("Chave nao informado")
code = self.detect_type_key()
if code == '0114':
Copy link
Owner

Choose a reason for hiding this comment

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

Evitar a utilização de números mágicos como esse ao longo do código.

@Mostela
Copy link
Owner

Mostela commented Jul 12, 2022

Olá Vi seu repositório e vi que aceitava apenas chave telefone, fiz umas mudanças para aceitar outras. Sou iniciante em python então aceito sugestões também

Que legal!!
Fico muito feliz pela contribuição sua 😄

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

2 participants