Python password generator

Question

This is considered as my second python project, I made the password generator where user has to input length of password minimum 5.

I made the code that it will guarantee at least one lowers case, one upper case and one special character is included in the password. please review and comment.

import random


def letters():
    return list('abcdefghijklmnopqrstuvwxyz')


def upper_case():
    uppercase = []
    for x in letters():
        uppercase.append(x.upper())
    return uppercase


def special_char():
    return list('!@#$%^&*()\'\"')

def all_char():
    return letters() + upper_case() + special_char()

def create_password(password, n):
    for x in range(n):
        password += random.choice(all_char())
    return password

def check_password(password):
    nletter, nupper, nspecial = [0]*3
    for x in password:
        if x in letters():
            nletter += 1
        if x in upper_case():
            nupper += 1
        if x in special_char():
            nspecial += 1
    if nletter > 0 and nupper > 0 and nspecial > 0:
        return True
    else:
        return False


def main():
    n = 0
    while n < 5:
        n = int(input('Enter your password length (minimum 5): '))
    password = ''
    while not check_password(password):
        password = ''
        password = create_password(password, n)
        check_password(password)

    print('Generated password : {}'.format(password))

while True:
    main()

Answer 1

To build on what else has been posted, validating your inputs is necessary and you have a lot of functions and loops that can be removed.

String module

First, I would like to introduce you to the string module that is part of the python standard library. This module has string constants that you can import and use:

>>> import string
>>> string.ascii_lowercase
'abcdefghijklmnopqrstuvwxyz'
>>> string.ascii_uppercase
'ABCDEFGHIJKLMNOPQRSTUVWXYZ'
>>> string.ascii_letters
'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'
>>> string.punctuation
'!"#$%&\'()*+,-./:;<=>?@[\\]^_`{|}~'

Using these would allow you to get rid of your first three functions (letter, upper_case, and special_char). Similarly to what Joe said, it would be better to make your character space a variable to avoid creating extra objects, and remove the all_char function at the same time.

create_password function

This function is clear and works okay, but can also be improved. As you are doing the same operation in each iteration of the loop (random.choice) this is and ideal scenario to use a list comprehension.

# Before
def create_password(password, n):
    for x in range(n):
        password += random.choice(all_char())
    return password

# After
def create_password(n):
    return ''.join([random.choice(all_char()) for _ in range(n)])

This works by creating a list where a random character is selected from all_char() and then joining them all together. The underscore is a 'throwaway' variable here, see here.

Alternatively, as we are already using the random module we could use random.choices (but only in python3.6 and above) as this will return a list of k size:

def create_password(n):
    return ''.join(random.choices(all_char(), k=n))

check_password function

In general, simple statements are better than complex statements. Therefore, I would change the first line of this function to be:

nletter, nupper, nspecial = 0, 0, 0

As this is clearer.

The for loop works, and could be improved by using the string module constants, though they would need to be made into lists.

for x in password:
    if x in list(string.ascii_lowercase):
        nletter += 1
    if x in list(string.ascii_uppercase):
        nupper += 1
    if x in list(string.punctuation):
        nspecial += 1

Another way to test if two strings overlap is to use sets; but this is getting more complex.

---

Altogether

import random
import string

def create_password(n, chars):
    return ''.join(random.choices(chars, k=n)) 

def check_password(password):
    nletter, nupper, nspecial = 0, 0, 0
    for x in password:
        if x in list(string.ascii_lowercase):
            nletter += 1
        if x in list(string.ascii_uppercase):
            nupper += 1
        if x in list(string.punctuation):
            nspecial += 1
    if nletter > 0 and nupper > 0 and nspecial > 0:
        return True
    else:
        return False

def main():
    characters = string.ascii_letters + string.punctuation
    n = 0
    while n < 5:
        try:
            n = int(input('Enter your password length (minimum 5): '))
        except ValueError:
            print('Not a number')

    password = ''
    while not check_password(password):
        password = create_password(n, characters)

    print('Generated password : {}'.format(password))

run = True
while run:
    try:
        main()
    except KeyboardInterrupt:
        run = False
        print('Ctrl+c caught exiting')

Answer 2

Lots of lists

Every time you call letters, upper_case, special_char or all_char, you are creating a new list which will be no different from the other lists that were previously created. You can make these variables that are assigned once and used everywhere.

Repeated checks

For every password you generate, you are checking its validity twice: once at the end of the loop, and the other on the while line. The former serves no purpose, and should be removed.

Unused assignment

The assignment to password = '' inside your while loop is unused, and can be removed.