6
\$\begingroup\$

I started university this semester. Can you review my code and share your thoughts with me

# 5-November-2020

username = input("Please enter your username: ")
password = input("Please enter your password: ")

username_password = {
    'Mert': "mertt"
}

if username in username_password:
    if username == "Mert" and password == username_password[username]:
        print("Welcome Sir")
    else:
        print("You're not authorized!")
else:
    confirmation = input("Do you want to register? (Y/N): ")
    if confirmation == "Y":
        username = input("Enter your username: ")
        password = input("Enter your password: ")
        password_2 = input("Confirm your password: ")
        if password == password_2:
            username_password[username] = password
        else:
            print("Your password is incorrect!")
    else:
        print("Good bye")
print(username_password)

If you have any improvement suggestion please tell me.

\$\endgroup\$
1
  • 4
    \$\begingroup\$ Welcome to code review! Please edit your title so that it only states what your code does. Also, an explanation of your code in the question body can also help reviewers! I suggest you read How to Ask \$\endgroup\$ – Parekh Nov 5 '20 at 14:23
3
\$\begingroup\$

Validate input

You should be certain that your program wouldn't fail for any kind of bad input, something the user shouldn't have entered that might cause the program to behave incorrectly

Is this possible for your program? Yes, take this line

username = input("Please enter your username: ")

If the user enters Ctrl+Z here, it causes an error and the program stops.

EOFError

We can catch these exceptions using try and except in Python

try:
    username = input("Please enter your username: ")
except (Exception, KeyboardInterrupt): 
    print("Please enter a valid username!")

The same applies to the input for password

One step further, I would also like to check the length of the details, i.e if the username or password is too short. Show a custom message if that is true


On an exception

Due to the changes we made previously, we can easily print an error message if we catch an exception rather than the termination of the program.
But what happens after?

You have two options, either to stop the program there because you cannot process bad input or, you can ask for input again.
If you choose the second option, that will take a few more lines of code but is surely better because the user gets another chance rather than running the whole program again For that, we need to wrap it in a while loop that will iterate till it gets valid input

def username_input():
    while True:
        try:
            username = input("Please enter your username: ")
        except Exception:
            print("Please a enter valid username! \n\n")
            continue 
        return username

def password_input():
    while True:
        try:
            password = input("Please enter your password: ")
        except Exception:
            print("Please enter a valid password! \n\n")
            continue
        return password

Here we are repeating ourselves, the functions are identical. With a little hack, we can clean up the code

def ask_input(field):
    while True:
        try:
            word = input(f"Please enter your {field}: ")
        except (Exception, KeyboardInterrupt):
            print(f"Please enter a valid {field}! \n\n")
            continue
        if not word: continue # if field is empty
        return word

def take_input():
    username = ask_input("username")
    password = ask_input("password")
    return username, password

Split work into functions

In my last point, you must've noticed I moved something into a function

We can really clean up our code if we follow the SRP rule or the Single-responsibility principle

The single-responsibility principle (SRP) is a computer-programming principle that states that every module, class, or function in a computer program should have responsibility over a single part of that program's functionality,

If assig the task of taking input in this program, to let's say take_input() We can re-use the same function if we would ever want to perform the same task again, without copy-pasting the same segment of code

username, password = take_input()
process_login( username, password ):
  • take_input() will take care of the input related tasks, and when it has proper valid it returns a username and password
  • process_login() decides whether the login will be authorized or not, based on the records

Check with key username

if username in username_password:
    if username == "Mert" and password == username_password[username]:
        print("Welcome Sir")
    else:
        print("You're not authorized!")

This works right now, but only due to the fact that the size of records is 1, any extra records added and your algorithm will fail, if the size was 50 you cannot write 50 if statements, you need to check with the username entered

def process_login(username, password, records):
    try:
        if records[username] == password:
            print("Authorized")
            return True
    except KeyError:
        print("Not authroized")
        return False

Here, if the username isn't present in records, a KeyError is raised, which I will catch and print Not authorized


Re-written

def ask_input(field):
    while True:
        try:
            word = input(f"Please enter your {field}: ")
        except (Exception, KeyboardInterrupt):
            print(f"Please enter a valid {field}! \n\n")
            continue
        if not word: continue

        return word

def take_input():
    username = ask_input("username")
    password = ask_input("password")
    return username,password


def process_login(username, password, records):
    try:
        if records[username] == password:
            print("Authorized")

    except KeyError:
        print("Not authroized")
        

I have left out the last part, which is the "registration". Which I will explain next

records = {
    "__Testu" : "__Testp",
    "__Testu2" : "__Testp2"
}

username, password = take_input()
process_login(username, password, records)

Write records into a file

As it is currently, you have pre-added a record to your dictionary, any record added after this is pointless since the next time you run the program, the dictionary will be initialized again with the same one record you added.
You need some way to save the users records so that the next time he opens your program, his previous details are saved

The best way to do this is the user the csv module in Python, - csv.DictReader which will automate all reading/writing of the file for you. But is something you would have to write on your own.

Cheers

>>> Please enter your username: (empty input)
>>> Please enter your username: __Testu
>>> Please enter your password: (empty input)
>>> Please enter your password: __Testp

>>> Authorized
\$\endgroup\$
10
  • \$\begingroup\$ I advise against the except Exception: approach written in your validation section. Ctrl+Z is a control character that exists for a reason. It's not meaningful to catch that and ask for another input. \$\endgroup\$ – Reinderien Nov 5 '20 at 15:50
  • \$\begingroup\$ In other words: yes, check for length and complexity, but do not intercept control characters. \$\endgroup\$ – Reinderien Nov 5 '20 at 15:50
  • \$\begingroup\$ @Reinderien Is it fine not to catch any exception here? \$\endgroup\$ – Parekh Nov 5 '20 at 15:51
  • \$\begingroup\$ @Reinderien The flag is only raised if the user deliberately presses Cntrl + Z, but the bare input of ^Z doesn't raise the exception \$\endgroup\$ – Parekh Nov 5 '20 at 15:55
  • \$\begingroup\$ Ctrl+Z and ^Z are the same thing? \$\endgroup\$ – Reinderien Nov 5 '20 at 15:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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