Gfrktyl

How can I reduce the amount of if statements in this program to make it more robust and any other tips for improvement would be grateful.

# importing pickle

import pickle



# defining variables

create_username = 0

create_password = 0

password = 0

username = 0



# this variable allows the user or denies the user to the rest of the program (will only run if access is 1)

access = 0





# creates a users dictionary

with open('users.pickle', 'rb') as f:

    users = pickle.load(f)



print(users)



# sign up (creating new account)

while username not in users and username != 'signup':

    username = input("enter username(type signup to create an account): ")



    # add new user to dictionary

    if username == "signup" or username == "Signup":

        create_username = input("enter a new username: ")

        create_password = input("enter a new password (Your password cannot be the same as your username !!!!!!!): ")



    if create_password in users:

        create_password = input("password taken re-enter: ")

    # then adds the new username to the users dictionary

    if username == 'signup':

        users[create_username] = create_password



if username in users:

    password = input("enter password: ")



if password in users:

    print("access granted")



    access = 1

if username not in users and username != 'signup':

    username = input("enter username: ")



    if username in users:

        password = input("enter password")



    if password in users:

        print("access granted")

        access = 1



if password not in users:

    print("access denied")



with open('users.pickle', 'wb') as f:

    # Pickle the 'data' dictionary using the highest protocol available.

    pickle.dump(users, f, pickle.HIGHEST_PROTOCOL)



print(users)

# importing pickle

Obviously. Don't clutter your code with comments unless they add something we don't already know.

# defining variables

create_username = 0

create_password = 0

password = 0

username = 0

This is generally a bad idea, and you aren't in mid-1990s C. Don't predeclare your variables. Initialize them where they're actually used.

print(users)

If users is a plain-old dictionary, I recommend the use of pprint instead of print.

"enter username(type signup to create an account): "

This is called in-band control, and is a bad idea. What if someone's username is called "signup"? You want out-of-band control. Ask explicitly whether the user wants to sign up or log in.

if username == "signup" or username == "Signup":

Don't do two comparisons. Convert username to lowercase and compare with 'signup'.

Bugs

Both your check for an existing user and checking if the user entered the correct password are not working correctly:

if create_password in users:

    create_password = input("password taken re-enter: ")

This just checks if the just entered password is also a username already in use. in for dictionaries checks only if a value is present in the keys of the dictionary, not the values.

But even if this would be working like you intended (checking if this is the password of another user), checking if another user has the same password is very dangerous. First of all it gives away the fact that you are saving your passwords in clear text, which you shouldn't, anyway. In the best case the user will not want to use your system for that reason alone, in the worst case they will want to crack it.

And for this second case there is even more information now, because now they know the password of one other user! They just don't know which one, but if they keep on trying passwords and save them whenever they are already taken, they get a nice dictionary of passwords, where they know that each one is some users password. If they want to crack some specific users account, they just try all of them.

if username in users:

    password = input("enter password")



if password in users:

    print("access granted")

    access = 1

This checks if the user exists (good) and if another user with the username of the just entered password also exists, but not if the password is the password of the user who wants to login (bad).

This means that you can login to your system by knowing a single username! Just enter the username as both the username and the password and you are guaranteed access.

Instead, you want to check if the password is that users password:

if username in users:

    password = input("enter password: ")

    if password == users[username]:

        print("access granted")

        access = 1

This also makes sure that the check for the password is only performed if the user actually entered a password. Otherwise it will check if the user 0 exists in users (in your current version) or it will raise a KeyError, since user[username] does not exist (in this version where the if is not nested).

Style

Using 1 and 0 for logged-in and not logged-in, respectively, sounds like you will mess this up at least once. Instead used logged_in = False in the beginning and then later set it to logged_in = True if the login is successful.

Indeed, you should probably move all of your code to separate functions, one of which, the login function, returns True or False depending on whether or not the login was successful.