# Simple Random Password Generator in Python

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

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
break
print("Good bye!")
sys.exit()
break
while GeneratePasswordAgain not in YesOptions or NoOptions:
print("Not a valid response! Try again.")
break
print("Good bye!")
sys.exit()
while GeneratePassword not in YesOptions or NoOptions:
print("Not a valid response! Try again.")
break
print("Good bye!")
sys.exit()
break
while GeneratePasswordAgain not in YesOptions or NoOptions:
print("Not a valid response! Try again.")
break
print("Good bye!")
sys.exit()
while Name is not Name.isalpha:
print("Not a valid response! Try again.")


• Why does the program for the user name at all? Why does it matter if the user name is "Martin", "Joe123" or "💩"? Dec 2 '17 at 8:32
• Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. Dec 2 '17 at 14:38

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)))

passwordCharacters = string.ascii_letters + string.digits + string.punctuation


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.

• When I don't use sys.exit() in while GeneratePassword in NoOptions: the while GeneratePassword not in YesOptions and NoOptions: snippet is executed though. Dec 2 '17 at 14:16
• Then that's another bug. Hint: What do you intend to accomplish with GeneratePassword not in YesOptions and NoOptions clause? What is the order of operations? What is the effect of using and between two lists? Dec 3 '17 at 7:41

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 :)

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__':


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

min_length: int = 8,
max_length: int = 16,
+ string.digits
+ string.punctuation)
) -> None:

question = f'{username} would you like to generate a random password? '
while True:

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

print('Good bye!')
return

while True:
print('Not a valid response! Try again.')

*,
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__':

• (Welcome to CR!) (I'd change the loop-statement in ask_name() to just if username.isalpha(): return username print('Not a valid response! Try again.').) Dec 3 '17 at 23:00