This is my first project using Python. I made a simple password generator that checks user input. How can I improve it?

import random
letters = ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u",
"v", "w", "x", "y", "z"]
# Input the length of the password
while True:
print(f"{password_length_input} is not a number, try again:")
continue
else:
if 6 <= password_length <= 15:
break
else:
print("The password must be between 6 and 15 characters, try again:")
continue
# Input the amount of numbers in password
while True:
print(f"{password_numbers_input} is not a number try again")
continue
print(f"The value is too high, choose maximum {password_length} numbers")
continue
else:
break
# Check for numbers and letters in password
while True:
break
else:
break



Usage example:

Choose the length of your password with numbers between 6 and 15:
Choose the amount of numbers you want in your password, max 8
Your password will be 8 characters with 2 numbers and 6 letters.
pzc11bmf

• Are you required to ask the user interactively for the 2 pieces of information? I ask because a more typical design for a password creation script would just get the required parameters directly on the command line and then just print the password, with no interactivity at all: python create-password 8 2
– FMc
Sep 11 '20 at 8:02
• Note: random should not be used for security purposes, but you can use secrets instead; you probably should replace random with secrets.SystemRandom() if generating real passwords or any other cryptographically secure value. Sep 11 '20 at 12:27

letters = ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u",
"v", "w", "x", "y", "z"]


This method of writing the alphabet is very error prone. I would import string and use string.ascii_lowercase in place of letters. If you want to generate your own range of letters for whatever reason, I would write

letters = [chr(n) for n in range(ord('a'), ord('z') + 1)]


since there's then no danger of omitting or duplicating a letter.

password_length = 0


These default values are never used. The defaults for password_numbers and password_letters don't make sense since those variables hold numbers. I would delete all three lines.

if not password_length_input.isnumeric():
print(f"{password_length_input} is not a number, try again:")
continue
else:


try:
except ValueError:
print(f"{password_length_input} is not a number, try again:")
continue


while True:
...
break
else:
...
break


There is no sense in having a while loop here since you always break out of it on the first iteration.

range(0,password_numbers)


You can just write range(password_numbers).

password.append(random.randrange(0,9))


This will append a digit from 0 to 8 inclusive, never 9. If you want all ten digits you should write random.randrange(10). Or, perhaps better, use random.choice(string.digits).

password_string = ''.join([str(item) for item in password])


If you use string.digits then every element of password will be a character so you can simplify this to password_string = ''.join(password).

A more straightforward way of generating a random string:

import random
import string

def get_random_string(length):
letters = string.ascii_lowercase
result_str = ''.join(random.choice(letters) for i in range(length))
print("Random string of length", length, "is:", result_str)

get_random_string(8)
get_random_string(8)
get_random_string(6)


borrowed from here, and there are more examples.

Now if you have specific requirements like a minimum number of digits, you can either tweak the formula, or generate two lists and merge them while shuffling the values.

There is an example in the link I quoted above: "Generate a random alphanumeric string with a fixed count of letters and digits" => merging two list comprehensions.

The way you are doing it is procedural but not Pythonic. It is kinda reinventing the wheel.

At the very least, your list of allowed characters should look like this:

letters = "abcdefghijklmnopqrstuvwxyz"


Then you pick out a random letter eg letters[3] will return 'd' since the list is 0-based and Python treats strings as sequences of characters. Using shuffle like you are already doing, you can write more concise code.

First

I suggest creating separate methods for each while loops.

Second

"while True" loop is not a good practice, I think. Instead of that, use condition.

Third

 class PasswordGenerator():

...

...
*
*
*


For the end remember to create functions with one responsibility. After that, you can write unit tests for each of that and it will be easier.

I definitely don't recommend using random for generating secure passwords. The reason being random is predictable and anyone can guess the next set of passwords that it is going to generate. So it is better to replace random with secrets and use secrets.SystemRandom()

Also, note one more flaw as you are using random.choice() method it can repeat the characters while generating. So if you don’t want to repeat the characters and still want to use random, then use random.sample() method.

If you are looking for a secure and robust password, Python has a module called as secrets, and you could utilize this to generate a random secured password.

The algorithm used by secrets is less predictable when compared to the random string module generation.

import secrets
import string


• Your assertion that all of random is predictable is incorrect. If you read secrets' source then you'll see secrets.SystemRandom is random.SystemRandom`. Oct 3 '21 at 23:41