0

I am working on the below password generator and an issue that baffles me.

There's a nested while loop which asks the user if they would like to select a new password after one is generated. Each time you selected "Yes", it should ideally generate a new password to replace the old one. It is not the case, with every re-selection, it adds a new password to the old one.

What am I not doing right here?

import random
from string import ascii_lowercase, ascii_uppercase, digits

special_chars = "!#$%&*@^"
available_chars = list(ascii_lowercase) + list(ascii_uppercase) + list(digits) + list(special_chars)


def get_length():
    user_l = ""
    while not user_l.isdigit() or int(user_l) < 6:
        user_l = input("Please input a password length.\n")
    return int(user_l)
    print(int(user_l))

def pwd_gen(length):
    return [random.choice(available_chars) for i in range(length)]


def pwd_chk(length):
    pwd = []
    while True:
        pwd = pwd_gen(length)
        if set(pwd) & set(ascii_lowercase) == set():
            continue
        elif set(pwd) & set(ascii_uppercase) == set():
            continue
        elif set(pwd) & set(digits) == set():
            continue
        elif set(pwd) & set(special_chars) == set():
            continue
        else:
            print("\nYour password is " + "".join(pwd))
            while True:
                accept = input("Would you like a different password? Y/N\n\n")
                if accept.lower() == "n" or accept.lower() == "no":
                    print("\nYour password is:")
                    break
                elif accept.lower() == "y" or accept.lower() == "yes":
                    pwd_chk(length)
                    break
                else:
                    print("\nInvalid input. Your password is " + "".join(pwd))
                    continue
            break

    pwd = "".join(pwd)
    print(pwd)

pwd_chk(get_length())
Improve this question
4
  • 3
    Calling itself recursively each time the user accepts doesn't make sense. The stack keeps growing for no reason. Commented May 24, 2020 at 21:13
  • 1
    Not directly related to your question, but you may want to consider simplifying some of your if/else statements by joining conditions, like the first one can turn into if not set(pwd) or not (set(ascii_lowercase) == set() | set(ascii_uppercase) == set() | set(digits) == set() | set(special_chars) == set()):. You can then get rid of all the continue statements. Commented May 24, 2020 at 21:24
  • @TomKarzes Thanks Tom. How do you get rid of recursive calls and still get the program to regenerate a password? Commented May 24, 2020 at 21:45
  • @LCY Instead of calling itself recursively, just have it loop. Commented May 24, 2020 at 22:07

2 Answers 2

Reset to default
3

You are not stopping the loop after you choose "Y", so the unfinished functions continue their execution. By using a return statement (rather than break) you stop the function from printing the password it generated.

import random
from string import ascii_lowercase, ascii_uppercase, digits

special_chars = "!#$%&*@^"
available_chars = list(ascii_lowercase) + list(ascii_uppercase) + list(digits) + list(special_chars)


def get_length():
    user_l = ""
    while not user_l.isdigit() or int(user_l) < 6:
        user_l = input("Please input a password length.\n")
    return int(user_l)
    print(int(user_l))

def pwd_gen(length):
    return [random.choice(available_chars) for i in range(length)]


def pwd_chk(length):
    pwd = []
    while True:
        pwd = pwd_gen(length)
        if set(pwd) & set(ascii_lowercase) == set():
            continue
        elif set(pwd) & set(ascii_uppercase) == set():
            continue
        elif set(pwd) & set(digits) == set():
            continue
        elif set(pwd) & set(special_chars) == set():
            continue
        else:
            print("\nYour password is " + "".join(pwd))
            while True:
                accept = input("Would you like a different password? Y/N\n\n")
                if accept.lower() == "n" or accept.lower() == "no":
                    print("\nYour password is:")
                    break
                elif accept.lower() == "y" or accept.lower() == "yes":
                    pwd_chk(length)
                    return # CHANGE THIS FROM "break" TO "return"! #
                else:
                    print("\nInvalid input. Your password is " + "".join(pwd))
                    continue
            break

    pwd = "".join(pwd)
    print(pwd)

pwd_chk(get_length())

As a side note, you shouldn't use the default random module to generate passwords if you will actually be using them. It is safer to use the secrets module, or seed the random number generator with

import os
import random
random.seed(os.urandom(16))

to achieve security.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

+1 for the side note. Returning after the recursive call will solve the printing issue, but as Tom Karzes commented, it's better not to make the recursive call anyway, since the stack will grow without much benefit. The answer posted by Sebastian starts to address this issue.
Thank you all ! I find the forum more amazing than the coding itself.
2

You could add a flag instead of invoking the function again (code of inner while):

            while True:
                replace_password = False # Flag added here
                accept = input("Would you like a different password? Y/N\n\n")
                if accept.lower() == "n" or accept.lower() == "no":
                    print("\nYour password is:")
                    break
                elif accept.lower() == "y" or accept.lower() == "yes":
                    replace_password = True # Setting flag
                    break
                else:
                    print("\nInvalid input. Your password is " + "".join(pwd))
                    continue
            if replace_password: # Action based on flag
                continue
            break
Improve this answer

2 Comments

This code still calls pwd_chk again, seemingly at odds with your proposal. Is that intentional?
Sorry for that it should not be there. That's probably enough stack for today...

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.