Return to Revisions

Remarks:

• Separate user interaction (I/O) and algorithm logic.
• Use a function to make your code reusable and abstract away complexity.
• Add type hints to your function and check them with pyright.
• Don't use f-strings unless there's actually interpolation happening.
• Don't assume input is valid; handle errors gracefully.
• Avoid indexing--prefer random.choices to select random items from a list (random.choice if you just want to select one item).
• There is no performance issue with your code, so "optimization" doesn't matter at this point. Focus on coding style and writing maintainable code.
• Add docstrings.
• Consider adding unit tests. Since your code uses randomness, you can use a regex or logic to make sure the password has the right stuff in it, or seed the random library to be deterministic.

Here's a rewrite:

from random import choices, shuffle


def generate_password(nr_letters: int, nr_numbers: int, nr_symbols: int) -> str:
    """Generates a password with n random letters, numbers and symbols"""
    letters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
    numbers = "0123456789"
    symbols = "!#$%&()*+"
    password = [
        *choices(letters, k=nr_letters),
        *choices(numbers, k=nr_numbers),
        *choices(symbols, k=nr_symbols),
    ]
    shuffle(password)
    return "".join(password)


def read_int(prompt: str, invalid: str = "Invalid integer input") -> int:
    """Reads an integer input from stdin, repeating until successful"""
    while True:
        try:
            return int(input(prompt))
        except ValueError:
            print(invalid)


def main():
    """Interacts with the user to generate a password"""
    print("Welcome to the PyPassword Generator!")
    nr_letters = read_int("How many letters would you like in your password? ")
    nr_symbols = read_int("How many symbols would you like? ")
    nr_numbers = read_int("How many numbers would you like? ")
    password = generate_password(nr_letters, nr_numbers, nr_symbols)
    print(password)


if __name__ == "__main__":
    main()