A password generator

Question

This a small project for a beginner, it basically generate symbols digits letters depending on the user input I want to make the code better and learn from my mistakes.

import string
import random
asklenght = 0
asku = int(input("Please choose a method:\n 1)Digits \n 2)Letters \n 3) Symbols \n 4) All \n "))
asklenght = int(input("How long do you want your password to be ? "))
 
digits = string.digits
letters = string.ascii_letters
symbols = string.punctuation
if asku == 1:
 outputpass = random.choice(digits)
elif asku == 2:
 outputpass = random.choice(letters)
elif asku == 3:
 outputpass = random.choice(symbols)
else:
 outputpass = random.choice(digits)
 outputpass = random.choice(letters)
 outputpass = random.choice(symbols)
 
for i in range(asklenght - 1 ):
 if asku == 1:
 outputpass += random.choice(digits)
 elif asku == 2:
 outputpass += random.choice(letters)
 elif asku == 3:
 outputpass += random.choice(symbols)
 else:
 outputpass += random.choice(digits)
 outputpass += random.choice(letters)
 outputpass += random.choice(symbols)
 
print(outputpass)

• Andrew Holmgren: I don't have any feedback beyond what FMC already said. I built a password generator, if you want something else to reference to github.com/adholmgren/password_gen. It's not super polished, was just going for something personal/functional, but just to see some other ideas.
• Pear: Hello! but this doesn't give me required length of password I entered 10 in the digits but it gave me 27 length of password! need to fix that
• BackpainYT: @Pear Yeah, i thought it did comment again if you want the final code
• Pear: what do you mean by Comment again if you want the final code? does that mean Now i commented and You will update your code?
• Mast: No, it's better not to touch the code after reviews start coming in. It gets really confusing on who reviewed what code otherwise, even if you clearly mark it as such. If you've updated your code significantly and want a new review on the new code, post a new question.

Answer 1

One of the easiest ways to improve code is to reduce repetition. The most-repeated elements in your code are random.choice and outputpass. You can reduce bulk by importing what you need rather than importing a module containing what you need.

from random import choice
from string import digits, ascii_letters, punctuation

Another bulk-reducer is to choose more practical variable names. By practical, I mean names that are are more compact but no less informative to the reader. For example, in a script that generates a password, the context is quite clear, so you can get away with a very short variable name: pw is one sensible option. Similarly, a simple name like n is just as clear as asklenght, because in context everybody understands what n means.

On the subject of naming, what does asku mean? Not very much to me. But your input prompt text is very clear. A better name might be be method (which your text uses) or perhaps even chartype, which is more specific.

Your code has a bug for All: it creates a password 3x as long as it should be.

You don't need any special logic for n of 1, 2, or 3. Just set up the loop and let it handle everything. The key is to use n rather than n - 1.

for i in range(n):
 ...

The conditional logic inside the loop has only one purpose: selecting the relevant characters to include in the password. Anytime you have a situation like that, logic can often be greatly simplified by creating a data structure.

characters = {
 1: digits,
 2: ascii_letters,
 3: punctuation,
 4: digits + ascii_letters + punctuation,
}

That change drastically reduces the code inside the loop:

pw = ''
for i in range(n):
 pw += choice(characters[chartype])

If you want to impress your friends, you can even write it in one shot by using a comprehension:

pw = ''.join(choice(characters[chartype]) for i in range(n))

For usability, you might also consider changing the way that chartype works: instead of asking users to type a number, which requires a small mental translation from a meaningful thing (letters, numbers, symbols, all) to an abstract thing (1, 2, 3, 4), you could just let them type the first letter of the actual thing.

Also for usability, if the context is already clear, shorter messages are easier on users, because they can scan them very quickly.

A final change to consider is whether to subject your user to interrogation by a computer. I come from the school of thought that says humans tell computers what to do, not the other way around. In addition to being pro-human, that policy has many practical benefits. For example, isn't it annoying that everything time you edit the code and re-run it, you have to answer the same damn questions. A different approach is to take those instructions directly from the command line. Python ships with a module called argparse that would work well for a script like this, but you can skip that if you want and just use sys.argv directly.

You probably should do some input validation, but I'll leave that for you to pursue or for some other reviewer to comment on. Here's the code with those suggested changes.

from string import digits, ascii_letters, punctuation
from random import choice
from sys import argv
if len(argv) == 3:
 chartype = args[1]
 n = int(args[2])
else:
 prompt = 'Password characters:\n (D)igits\n (L)etters\n (S)ymbols\n (A)ll\n'
 chartype = input(prompt)
 n = int(input("Length? "))
 chartype = chartype.lower()[0]
characters = {
 'd': digits,
 'l': ascii_letters,
 's': punctuation,
 'a': digits + ascii_letters + punctuation,
}
pw = ''.join(choice(characters[chartype]) for i in range(n))
print(pw)

• BackpainYT: You are the same guy that helped me the other time, Thanks for all these tips, but i tried the programme in the terminal whenever i try to pass arguments it gives me an error in the terminal "Traceback (most recent call last): File ".../Password generator/propass.py", line 8, in <module> n = int(args[1]) IndexError: list index out of range " you don't need to help me with this, its a little bit advanced for me but i'm curios if you have time, thanks.
• BackpainYT: the programme work fine in my IDE by the way.
• BackpainYT: ah, sorry i didn't know i should put all the arguments, i appreciate your help i truly do <3
• FMc: @BackpainYT Probably better to use if len(argv) == 3 than my initial code. I edited that. And yes, you can put arguments on the command line when you run a script. That's the standard way to interact with these kinds of programs and a useful technique in many situations.
• Toby Speight: We could be nicer to the user when invalid inputs are supplied.

Answer 2

Don't use random.choice() for generating passwords. As the documentation says:

Warning: The pseudo-random generators of this module should not be used for security purposes. For security or cryptographic uses, see the secrets module.

---

I'm not sure what you're expecting from these three lines:

outputpass = random.choice(digits)
outputpass = random.choice(letters)
outputpass = random.choice(symbols)

The first two have no effect, since their results immediately get overwritten. That's bad, as it's giving us much less password entropy than the user asked for.

Probably you meant to choose from all three sets:

outputpass = random.choice(digits + letters + symbols)

Similarly, where we have

 outputpass += random.choice(digits)
 outputpass += random.choice(letters)
 outputpass += random.choice(symbols)

we're appending three characters, one from each set.

As FMc's answer says, don't repeat these - just start with an empty string.

---

Minor (spelling): length, not lenght.

• Peilonrayz: Note: secrets is just a small wrapper around random.SystemRandom.
• Toby Speight: True; I've made my first sentence more specific - it now agrees with the quoted text.

Answer 3

FMC and Toby have already added some excellent answers. So, I'll just build on what FMC said and add a small change.

In Python 3.6, a new function called choices (https://docs.python.org/3/library/random.html#random.choices) was added. It basically lets you pick k different values from n different choices. You can use this to skip calling random.choice in loop.

from string import digits, ascii_letters, punctuation
from random import choices
from sys import argv
if len(argv) == 3:
 chartype = args[1]
 n = int(args[2])
else:
 prompt = 'Password characters:\n (D)igits\n (L)etters\n (S)ymbols\n (A)ll\n'
 chartype = input(prompt)
 n = int(input("Length? "))
 chartype = chartype.lower()[0]
characters = {
 'd': digits,
 'l': ascii_letters,
 's': punctuation,
 'a': digits + ascii_letters + punctuation,
}
pw = ''.join(choices(characters[chartype], k=n))
print(pw)

• BackpainYT: I encountered this function. I didn't know how to use it, but thanks for the advice, i really appreciate it .