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

Ejercicio Dojo PEP y buenas practicas #2

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

Conversation

Underdoge
Copy link

@Underdoge Underdoge commented May 24, 2023

Como en la descripción no se habla del número de intentos máximo, sino que simplemente se debe de adivinar el número en el menor número de intentos posibles, eliminé los intentos totales del main, y permito que el usuario intente indefinidamente hasta que introduzca una "q" para terminar. De otra manera se debería especificar que el usuario solo tiene 10 intentos para adivinar el número.

También me di cuenta que el modulo codebreaker.py originalmente definía una constante con un número de cuatro dígitos pero tenía dígitos repetidos lo cual contradice la definición del juego, por lo que mejor genero un número aleatorio de 4 dígitos sin dígitos repetidos cada que se instancía la clase.

El código fue probado contra autopep8 y flak8, los cuales se agregaron al pre-commit hook y no arrojan errores al hacer commit.

@camigomezdev
Copy link
Owner

Gran Trabajo 👏🏼

codebreaker.py Outdated
"""
self.right_number = str(''.join(random.sample("0123456789", 4)))

def guess(self, number=None):
Copy link
Owner

Choose a reason for hiding this comment

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

Este nombre podría ser mas explicito para entender que hace la función en su totalidad.

Copy link
Author

Choose a reason for hiding this comment

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

listo, gracias!

@Underdoge
Copy link
Author

Gran Trabajo 👏🏼

mil gracias!

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.

2 participants