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

Feat: Aplica PEP 8, añade hook pre-commit y modifica el proyecto #16

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

Conversation

joselofierro
Copy link

Happy path

  1. Clona el repositorio: git clone https://github.com/joselofierro/dojo_pep8.git
  2. Ubicate en la carpeta: cd dojo_pep8
  3. Crea un entorno virtual virtualenv -p /usr/bin/python3 venv
  4. Activa el entorno virtual source venv/bin/activate
  5. Instala los paquetes pip install -r requirements.txt
  6. Ejecuta el script python main.py

codebreaker.py Outdated
elif number[idx] in NUMBER_TO_GUESS:
resultTrack += '_'
else:
resultTrack += '#'
Copy link
Owner

Choose a reason for hiding this comment

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

Puedes modificar la logica para que agrupe primero las (X), luego los (_) bajos y por ultimo los (#)?

Copy link
Author

Choose a reason for hiding this comment

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

Aplico cambios

codebreaker.py Outdated
if number == NUMBER_TO_GUESS:
return True

resultTrack = ''
Copy link
Owner

Choose a reason for hiding this comment

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

Recuerda que PEP8 recomienda utilizar snake_case en vez de camelCase para el nombramiento de las variables

Copy link
Author

Choose a reason for hiding this comment

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

Aplico cambios

codebreaker.py Outdated

return resultadoX+resultado_

def adivinar(self, number: str = None) -> Union[bool, str]:
Copy link
Owner

Choose a reason for hiding this comment

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

Puedes dar un nombre mas explicito a esta función?

Copy link
Author

Choose a reason for hiding this comment

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

Aplico cambios

codebreaker.py Outdated
else:
resultTrack += '#'

return resultTrack
Copy link
Owner

Choose a reason for hiding this comment

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

Estas mezclando español e inglés en los nombres de tus variables, puedes unificarlo en un solo idioma?

Copy link
Author

Choose a reason for hiding this comment

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

Aplico cambios

main.py Outdated
@@ -1,18 +1,17 @@
from codebreaker import Codebreaker

intentos_totales = 10
TOTAL_ATTEMPTS = 5
ATTEMPTS = 1
Copy link
Owner

Choose a reason for hiding this comment

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

attepts funciona como una variable ya que en cada iteración cambia su valor, escribirlo en mayuscula indicaría que es una constante y que siempre su valor será 1

Copy link
Author

Choose a reason for hiding this comment

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

Aplico cambios

main.py Outdated
number = input('Adivina el número de 4 cifras: ')
resolve = codebreaker.adivinar(number)
print(resolve)
if resolve is True:
Copy link
Owner

Choose a reason for hiding this comment

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

Puedes modificar este if para que solo pregunte if resolve:, si resolve puede ser otros tipos de datos que tambien son funcionan como verdadero trata de modificar el codigo para que siempre sea un valor booleano.

Copy link
Author

Choose a reason for hiding this comment

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

Aplico cambios

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