# Script to generate random password

I made this random password script. It works, but i think it is kind of bloated. The part where random characters are being added is the area where I think it needs work. I don't want to declare if and else statement for every combination of lowercase list, uppercase list and digits list(I am taking input from user whether they want uppercase,lowercase,digits or any possible combination of these three). Is there any possible way in which I can just check which of these three values are yes and then just use only those.

Also what is more efficient using lower() to turn the list called using string module into lower case or just calling the lowercase list itself.

import random
from  string import ascii_letters ,digits
while True:
userchoice_syntax_lower = input("Do you want lower case letter?(y or n) ")
userchoice_syntax_upper = input("Do you want upper case letter?(y or n) ")
userchoice_syntax_number = input("Do you want numbers?(y or n) ")
counter=0
choice_list=[1,2,3]
while counter <= userchoice_size:
choice_number = random.choice(choice_list)
if choice_number == 1:
if userchoice_syntax_upper == 'y': # Uppercase list
counter += 1
if choice_number == 2:
if userchoice_syntax_number=='y': # Digits list
counter += 1
if choice_number == 3:
if userchoice_syntax_lower == 'y': # Lower case list
password= password + str(random.choice(ascii_letters.lower())) # Converts all letters into lowercase then randomly select and store in list
counter += 1


## Style

Your style is mostly fine, except lacking a bit of white space, especially after the imports, and having mostly useless comments that break the 80 character barrier.

## Input validation

You don't check if the user input can be parsed as an integer in the first input. The int() methods validates its input, however a bad input will crash your program. It would be preferable to reject the input, and ask again.

You don't validate the input for the yes/no question either, and it is much worse. Typing anything other than a lowercase y will be treated as a no. At the very least, add a .lower() to handle inputs like Y. It would be high priority to check for n/N, instead of silently treating anything else than y as meaning no.

Since you ask 3 yes/no questions (and possibly more if you'd like to add stuff like special characters), you should delegate the input and its validation to a separate method.

## Bug

If the user says "no" (more precisely: not "y") to all 3 questions, the program will hang indefinitely, as counter will never increase and the inner while loop will never end.

## Security

random is not a cryptographically secure random number generator. Use secrets instead, which is very similar to use.

Also, ideally you'd want the probability for any character to be chosen to be the same. By separating the character categories as you did, digits are over-represented if they appear alongside letters.

## Outer while loop

You put the entire code in an infinite loop. How many passwords do you think a user would want to generate?

# My take on the problem

def generate_password():

choices = ''
choices += string.ascii_lowercase
choices += string.ascii_uppercase
choices += string.digits

while True:
print(msg)
try:
return int(input('> '))
except ValueError:
print('Unable to parse number. Please try again.')

while True:
print(msg)
user_in = input('> ').lower()
if user_in =='y':
return True
if user_in =='n':
return False

• password += should be replaced with ''.join - secrets even shows an example doing so. Jun 19 '20 at 16:23
• @Reinderien Is there a reason - besides performance - for doing so? I don't like list comprehensions, as they are rather hard to read. I assume that arsh is quite new to python, and I feel that code readability is more valuable than a clever one-liner, especially at a beginner level. Jun 19 '20 at 16:31
• The use of ''.join in this case is already well-understood, and is more "standard" than "clever". Terseness and performance are both factors, and for some it's more legible than a loop. For the scale of password generation performance will not matter, but in other applications your loop will be O(n²) where join is O(n). Jun 19 '20 at 16:33
• More generally: I try to give submissions on this site the best code possible and not necessarily code that they're guaranteed to understand, since presumably they came here to learn. People that come here are aspiring. Jun 19 '20 at 16:36

## Character distribution

You randomly choose between uppercase, lowercase and digit characters with an equal probability, but there are more letters than digit characters. That means characters will be under-represented in your password's probability distribution. Easier and more accurate is to make one string of all eligible characters and choose from it randomly.

## Redundant str

No need for str here:

str(random.choice(ascii_letters.upper()))


counter += 1


should not be repeated three times in if statements. You can do it once at the end of the loop. Better yet,

for _ in range(userchoice_size):


## Choice list

This:

choice_list=[1,2,3]
choice_number = random.choice(choice_list)


should not use a list, and should be replaced with a call to randrange.

## Secure entropy

For password generation it's crucially important to use a secure source of entropy. Read this quote carefully:

Python uses the Mersenne Twister as the core generator. It produces 53-bit precision floats and has a period of 2**19937-1. The underlying implementation in C is both fast and threadsafe. The Mersenne Twister is one of the most extensively tested random number generators in existence. However, being completely deterministic, it is not suitable for all purposes, and is completely unsuitable for cryptographic purposes.

Emphasis mine. A better built-in option is secrets.