3
\$\begingroup\$

I am very new to coding and I have created this relatively simple code to do with my basic knowledge of the language. However I have had to do with some unwanted code including shutting it down if password is too long. so could anybody help me with any improvements to this?

import time

passwordx = float(input('create your password'))
def password():
    passwordx
    if passwordx>9999:
        print('password is wrong\n innitiating shutdown')

        x=5
        time.sleep(1)
        print(x)
        x-=1
        time.sleep(1)
        print(x)
        x-=1
        time.sleep(1)
        print(x)
        x-=1
        time.sleep(1)
        print(x)
        x-=1
        time.sleep(1)
        print(x)
        x-=1
        time.sleep(1)
        print(x)
        x-=1
        quit()



    else:
        print('code saved')
        x=1
        while x <1000:
            print('###############')
            x+=1

def login():
    passwordguess = float(input('please enter password'))


    if passwordguess == passwordx:
        print('access granted')

    else:
        print('wrong')
        print('here\'s a clue', (passwordx-passwordguess)**2)
        login()

password()

login()
\$\endgroup\$
1
  • \$\begingroup\$ Just to bare in mind I added the clue bit so I would always know what the code is and trick other people and get their code right all the time \$\endgroup\$
    – J.Fitz
    Commented Jun 25, 2016 at 18:45

1 Answer 1

2
\$\begingroup\$

A couple of things:

  1. Start using range.

    Range is the recommended way to loop in Python. And you can change your shutdown sequence, and while loop to use it. This improves the clarity of your code, which is good for when others read it. And even if no-one else will, it's always good to get good habits.

  2. Reduce the amount of unneeded messages to the end user. Here less is more.

    Seriously, I was used to overly verbose messages when I was on windows. And then I installed Linux and would most of the time get no feedback. To change my keyboard layout I use loadkeys dvorak, and I get no message. I create a file, touch file, and again no message. The only time I get a message is if that's the intent of the command, ls, or if it's an error.

    Honestly you should probably take a leap out of their book.

  3. It's normally best to avoid recursion.

    I once made a program that heavily used recursion, it exceeded the recursion limit and there was no way for me to fix it. I can assure you I never touched that project again.

    Instead I'd use a while loop.

  4. Use getpass.

    This removes the need to display a lot of hashes to remove the password from display. As it never displays in the first place.

  5. Change password to create the password.

    This way the method of creating a password is always the same.

  6. Change login to take the password as an argument.

    This is as it removes reliance on globals which is always a good thing.

  7. You should check the input is a number, otherwise you program will exit with an error.

from time import sleep
from getpass import getpass # 4

def create_password(): # 5
    password = getpass('Create your password: ') # 4, 5
    try: # 7
        password = float(password)
    except ValueError:
        quit()
    if password > 9999:
        print('Password is invalid, initiating shutdown.')
        for i in range(5, -1, -1): # 1
            time.sleep(1)
            print(i)
        quit()
    return password # 5

def login(password): # 6
    while True: # 3
        password_guess = float(input('Please enter password: '))
        if password_guess == password:
            break # 3
        else:
            print("Wrong. Here's a clue", (password - password_guess) ** 2)

password = create_password()
login(password)

The above shows how I'd change your code if I were to keep the messages you added special code for. I still think most of your messages are not needed, and so I'd remove more of them. I also dislike your clue, as it's ridiculously insecure. And due to floating point inaccuracy I'd store the password as a string. Resulting in:

from getpass import getpass

def create_password(): # 5
    password = getpass('Create your password: ')
    try:
        float(password)
    except ValueError:
        quit()
    if float(password) > 9999:
        quit()
    return password

def login(password):
    while True:
        password_guess = input('Please enter password: ')
        if password_guess == password:
            break

password = create_password()
login(password)

Finally I'd think about removing the float(password) > 9999.

\$\endgroup\$
2
  • \$\begingroup\$ The issue with your code is that it doesn't separate the business logic from I/O. I'd much rather have validate_password instead of create_password (and use getpass() in main()), etc. \$\endgroup\$
    – Mahi
    Commented Jun 27, 2016 at 15:40
  • \$\begingroup\$ @MarkusMeskanen That's correct, I'll amend my answer. Thanks \$\endgroup\$
    – Peilonrayz
    Commented Jun 27, 2016 at 15:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.