2
\$\begingroup\$

So here I wanted to make a system that registered a person's account. It remembers the person's email, account, first and last names, by writing to a file. You can login to that account by typing the email and password according to that email and then you would get a hello message, saying your first and last name that you used to register your account with.

data_text = []
first_names = []
last_names = []
emails = []
passwords = []

def user_choice():
    print('Hello user, create an account or login to an existing one.')
    choice = input('Insert "1" if you wish to create an account or "2" if you wish to login: ')
    print('\r')
    if choice == '1':
        create_account()
        user_choice()
    else:
        login_account()
        user_choice()

def register_info():
    with open('Login_Data.txt', 'r') as login_data:
        global data_text, first_names, last_names, emails, passwords
        data_text = login_data.readlines()
        for i in data_text:
            data_text[data_text.index(i)] = i.strip()
        emails = (data_text[2::4])

def create_account():
    with open('Login_Data.txt', 'a') as login_data:
        first_name = input('First name: ')
        last_name = input('Last name: ')
        email = input('Insert your Email adress: ')
        while email in emails:
            print('That email is already registered')
            email = input('Insert another Email adress: ')
        password = input('Create a password: ')
        passwordc = input('Confirm your password: ')
        info = [first_name, last_name, email, password]
        while passwordc != password:
            print('The passwords do not match.')
            passwordc = input('Reinsert your password: ')
        for i in info:
            login_data.write(i)
            login_data.write('\n')
    print('Nice! Your account was registered.')
    print('\r')
    register_info()

def login_account():
    register_info()
    with open('Login_Data.txt', 'r'):
        login_email = input('Email: ')
        while login_email not in emails:
            print('Invalid Email')
            login_email = input('Reinsert your Email: ')
        login_password = input('Password: ')
        while login_password != data_text[data_text.index(login_email) + 1]:
            print('Invalid password')
            login_password = input('Reinsert your password: ')
        print('Hello {} {}, welcome back!'.format(data_text[data_text.index(login_email) - 2], data_text[data_text.index(login_email) - 1]))
        print('\r')
user_choice()
\$\endgroup\$
2
\$\begingroup\$
  • user_choice is your main loop, and so it would be better described as main. Whilst fairly undescriptive on what it does you can add a docstring to add information on what it performs.
  • It is more idiomatic to use loops in Python rather than recursion. This is partly due to the recursion limit, and partially that the iterator pattern has a lot of support in Python.
  • I've rarely seen '\r' in Python. I don't see why you would need it and so have just removed them.
  • I like the way that you got a users email. The use of a while loop here is pretty clean.
  • I would change the prompted text when getting an email as it's different to the rest of the prompts.
  • I would DRY your do while loop. There's not really a point to having two different input prompts with the above suggestion.
  • Recently I was creating an account using passwd and multiple times I kept messing up the first password entry. It was rather vexing as I knew I messed up and I just had to hope I pressed backspace the correct amount of times to fix my mess up. Unfortunately I wasn't skilled enough to fix my mistake.

    However with your solution I would have to quit out of your entire program, removing the data I have already to fix my mistake. You should instead get again the password and the confirmation.

  • There is no point in opening login_data until the user input has been confirmed to be correct. You should try to keep with statements as small as necessary.
  • I would prefer to build a dictionary or an abstract datatype for the user information.
  • You can remove one of your login_data.writes by passing i + '\n'.
  • "Login_Data.txt" seems ok, but it's not great. You can use JSON via import json and make a much easier and standard file type to work with.
  • Your interactions with your globals seem really poor. If you want a global, than I suggest you only have one. This is easy when using dictionaries.

    I would also suggest changing your code to have no globals.

  • I would change login_account to align with the above, changed do-while loops, no globals and using JSON.
  • Since we are using JSON we can easily change the format to display the first and last name easier.
  • Your solution is in no way safe, it's prone to over the shoulder attacks by not using getpass and you're storing passwords in plain-text. This is not safe. Do not use this for anything more than a toy project that will only ever have you entering a password unique to this toy.
import json


def main():
    """Create an account or log in."""
    while True:
        print('Hello user, create an account or login to an existing one.')
        choice = input('Insert "1" if you wish to create an account or "2" if you wish to login: ')
        print()
        if choice == '1':
            create_account()
        else:
            login_account()


def input_user_email(emails):
    """Get an unowned email."""
    while True:
        email = input('Email: ')
        if email not in emails:
            return email
        print('That email is already registered.')


def input_user_password():
    """Get a user's email."""
    while True:
        password = input('Password: ')
        confirmation = input('Confirm password: ')
        if password == confirmation:
            return password
        print('The passwords do not match.')


def get_user_information(emails):
    """Get a user's information."""
    return {
        'first_name': input('First Name: '),
        'last_name': input('Last Name: '),
        'email': input_user_email(emails),
        'password': input_user_password(),
    }


def create_account():
    """Create user account."""
    with open('login_data.json', 'w+') as f:
        users = json.load(f)
        emails = {user['email'] for user in users}
        user = get_user_information(emails)
        users.append(user)
        json.dump(users, f)
    print('Nice! Your account was registered.\n')


def login_account():
    """Log into a user account."""
    with open('login_data.json') as f:
        users = json.load(f)
    by_email = {user['email']: user for user in users}

    while True:
        email = input('Email: ')
        if email in by_email:
            user = by_email[email]
            break
        print('Invalid Email')

    while True:
        password = input('Password: ')
        if password == user['password']:
            break
        print('Invalid password')

    print('Hello {0.first_name} {0.last_name}, welcome back!\n'.format(user))


if __name__ == '__main__':
    main()
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for all your suggestions man, about the '\r' thing, I didn't know how to separate the data in different lines by using '.append'. I'll see if I can learn about encrypting text and JSON, thanks for mentioning that too. \$\endgroup\$ – Tlomoloko Jan 11 at 15:24
1
\$\begingroup\$

Conversion of a comment to an answer.

Definitely avoid the recursion in user_choice. It will run out of stack if you keep exercising it for long enough. A simple while True: around the rest of the function body should fix it (obviously take out the calls where it calls itself).
- tripleee

| improve this answer | |
\$\endgroup\$

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.