Simple Random Password Generator in Python

Question

I've made a password generator that works fine. It takes alphabetic, numeric, and punctuation characters and randomly generates a 8-16 character password.

import sys
import string
import random

def PasswordGenerator():
    Name = input("What is your name? ")
    if Name.isalpha():
        GeneratePassword = input(Name + " would you like to generate a random password? ")
        YesOptions = ["Yes", "yes", "Y", "y"]
        NoOptions = ["No", "no", "N", "n"]
        PasswordCharacters = string.ascii_letters + string.digits + string.punctuation
        while GeneratePassword in YesOptions:
            Password =  "".join(random.choice(PasswordCharacters) for i in range(random.randint(8, 16)))
            print(Password)
            GeneratePasswordAgain = input("Would you like to generate another random password? ")
            while GeneratePasswordAgain in YesOptions:
                Password =  "".join(random.choice(PasswordCharacters) for i in range(random.randint(8, 16)))
                print(Password)
                GeneratePasswordAgain = input("Would you like to generate another random password? ")
                break
            while GeneratePasswordAgain in NoOptions:
                print("Good bye!")
                sys.exit()
                break
            while GeneratePasswordAgain not in YesOptions or NoOptions:
                print("Not a valid response! Try again.")
                GeneratePasswordAgain = input("Would you like to generate another random password? ")
                break
        while GeneratePassword in NoOptions:
            print("Good bye!")
            sys.exit()
        while GeneratePassword not in YesOptions or NoOptions:
            print("Not a valid response! Try again.")
            PasswordGenerator()
            while GeneratePassword in YesOptions:
                Password =  "".join(random.choice(PasswordCharacters) for i in range(random.randint(8, 16)))
                print(Password)
                GeneratePasswordAgain = input("Would you like to generate another random password? ")
                while GeneratePasswordAgain in YesOptions:
                    Password =  "".join(random.choice(PasswordCharacters) for i in range(random.randint(8, 16)))
                    print(Password)
                    break
                    GeneratePasswordAgain = input("Would you like to generate another random password? ")
                while GeneratePasswordAgain in NoOptions:
                    print("Good bye!")
                    sys.exit()
                    break
                while GeneratePasswordAgain not in YesOptions or NoOptions:
                    print("Not a valid response! Try again.")
                    GeneratePasswordAgain = input("Would you like to generate another random password? ")
                    break
        while GeneratePassword in NoOptions:
            print("Good bye!")
            sys.exit()
    while Name is not Name.isalpha:
        print("Not a valid response! Try again.")
        PasswordGenerator()

PasswordGenerator()

Answer 1

I found a bug. If the initial response to "Do you want to..." is neither yes-like or no-like, it prompts for the username again. Also, you have large sections of dead code that will never execute.

1. Avoid duplicating code.

   Several prompts are exactly the same as each other, even with the same logic after them. Refactor the code so that each possibility only occurs once.

2. Avoid duplicating variables

   The GeneratePassword and GeneratePasswordAgain variables serve the same purpose: to get user input and determine if it should continue. and should be unified.

3. Keep the code reusable.

   Avoid calls like sys.exit() that prevent this snippet of code from being used elsewhere.

4. Use the language features properly.

   You know how to use while loops -- so why are you making multiple unnecessary recursive calls, each of which performs the same task as a while loop?

5. Separate logic and user-interface. Separate the code into modules.

   Write the function that generates the password and does nothing else. Something like

    def RandomPassword(passwordCharacters, minLetters, maxLetters):
       return "".join(random.choice(PasswordCharacters) for i in range(random.randint(minLetters, maxLetters)))
   
    def DefaultRandomPassword():
        passwordCharacters = string.ascii_letters + string.digits + string.punctuation
        return RandomPassword(passwordCharacters, 8, 16)
   

   Write another function that takes a prompt, displays it, waits for the user to type something, and then does it again if it was not a valid yes-no answer. Then return either yes or no. Call this function multiple times.

Answer 2

In addition to @Snowbody's answer, I'd like to point out that random.choice and random.randint are not fit for generating passwords. The random module makes use of the Mersenne Twister, which is not cryptographically secure. The Python 3.6 release added secrets to the standard library, which retrieves random data from /dev/urandom. If you're not running Python 3.6 or upwards, you can still use random.SystemRandom (or os.urandom directly).

---

Also, have a close look at this xkcd post :)

Answer 3

1. PEP 8: Top-level functions should be surrounded by two blank lines. CamelCase names are usually for classes. Functions and variables should be in lower_case_with_underscores. Limit all lines to a maximum of 79 characters.

2. Instead of PasswordGenerator() in the end of your script write this:

   if __name__ == '__main__':
       PasswordGenerator()
   

   You can read about it for example here.

3. If you have python3.6 then you can use f-strings:

   GeneratePassword = input(f"{Name} would you like to generate a random password? ")
   

4. You have some magic numbers in your code. How about taking them to function's signature? We also could take there YesOptions and NoOptions and use type hints. After all, explicit is better than implicit.
5. Instead of using i variables in your loops use _. More information here.
6. sys.exit() is too extreme. Use return instead.
7. Some lines of your code that go after sys.exit() and break are unreachable.
8. All this spaghetti code with recursions can be rewritten in a much clearer way. Check out this answer about asking user for a valid response.

In the end it can be something like this:

import random
import string
from typing import Set


def generate_passwords(*,
                       min_length: int = 8,
                       max_length: int = 16,
                       password_characters: str = (string.ascii_letters
                                                   + string.digits
                                                   + string.punctuation)
                       ) -> None:
    username = ask_name()

    question = f'{username} would you like to generate a random password? '
    while True:
        if user_wants_password(question):
            password_length = random.randint(min_length, max_length)
            password = ''.join(random.choice(password_characters)
                               for _ in range(password_length))
            print(password)

            question = 'Would you like to generate another random password? '
            continue

        print('Good bye!')
        return


def ask_name() -> str:
    while True:
        username = input('What is your name? ')
        if username.isalpha():
            return username
        print('Not a valid response! Try again.')


def user_wants_password(question: str,
                        *,
                        yes_options: Set[str] = {'Yes', 'yes', 'Y', 'y'},
                        no_options: Set[str] = {'No', 'no', 'N', 'n'}) -> bool:
    while True:
        response = input(question)
        if response in yes_options:
            return True
        elif response in no_options:
            return False
        else:
            print('Not a valid response! Try again.')


if __name__ == '__main__':
    generate_passwords()