Python object-oriented password generator

Question

I have made a random password generator using a class called password and a method called generate.

My program works as it should. It generates a random password determined by the users preferences for length, upper or lowercase, numbers and special characters.

I was just wondering if there was a way to refactor the numerous if statements I have used to determine what sort of password the program would generate.

Any other suggestions for improvements I could make would also be helpful. Thanks a ton :D

Code:

import random import string class password: def __init__(self, length, string_method, numbers=True, special_chars=False): self.length = length self.string_method = string_method self.numbers = numbers self.special_chars = special_chars def generate(self, iterations): # Checking what type of string method the user has asked for if self.string_method == 'upper': stringMethod = string.ascii_uppercase elif self.string_method == 'lower': stringMethod = string.ascii_lowercase elif self.string_method == 'both': stringMethod = string.ascii_letters # Checking if the user has asked for numbers or not if self.numbers == True: stringNumbers = string.digits elif self.numbers == False: stringNumbers = '' # Checking if the user has asked for special characters or not if self.special_chars == True: stringSpecial = string.punctuation elif self.special_chars == False: stringSpecial = '' characters = stringMethod + stringNumbers + stringSpecial # Generating the password for p in range(iterations): output_password = '' for c in range(self.length): output_password += random.choice(characters) print(output_password) # Test password1 = password(20, 'lower', True, False) # password length = 20, string method is lowercase, numbers are true and special characters are false password1.generate(3) # generate the random password 3 times 

Answer 1

You can use the ternary operation to reduce the vertical space, i.e. x = a if condition else b. This sets x to a if condition is true and to b otherwise.

You can also use a dictionary to map a string to some object, like

the_map = {'a': 4, 'b': 7, 'c': 43}` x = the_map['c'] 

This will set x to 43.

Then you can evaluate everything in the initializer instead. So it's not less if checks per se, but it's a bit cleaner, and you can call generate multiple times without needing to do the checks.

import random import string class Password: def __init__(self, length, string_method, numbers=True, special_chars=False): self.length = length self.string_method = { 'upper': string.ascii_uppercase, 'lower': string.ascii_lowercase, 'both': string.ascii_letters }[string_method] self.numbers = string.digits if numbers else '' self.special_chars = string.punctuation if special_chars else '' def generate(self, iterations): characters = self.string_method + self.numbers + self.special_chars # Generating the password for p in range(iterations): output_password = '' for c in range(self.length): output_password += random.choice(characters) print(output_password) # Test password = Password(20, 'lower', True, False) password.generate(3) # generate the random password 3 times 

If you actually want less checks, then just pass in the characters directly. Passing in the length and which characters you want to use into password generator seems perfectly legitimate and straight-forward. This also allows you to do more complex things like generating a password with just vowels, without even numbers, or maybe if you want to exclude certain punctuation.

import random import string class Password: def __init__(self, length, characters): self.length = length self.characters = characters def generate(self, iterations): # Generating the password for p in range(iterations): output_password = '' for c in range(self.length): output_password += random.choice(self.characters) print(output_password) # Test password = Password(20, string.ascii_lowercase + string.digits) password.generate(3) # generate the random password 3 times 

Answer 2

Since you eventually combine the different char sets anyway, you might as well do that right away, with one if per char set. And random.choices saves a loop.

class password: def __init__(self, length, string_method, numbers=True, special_chars=False): self.length = length self.chars = '' if string_method != 'lower': self.chars += string.ascii_uppercase if string_method != 'upper': self.chars += string.ascii_lowercase if numbers: self.chars += string.digits if special_chars: self.chars += string.punctuation def generate(self, iterations): for _ in range(iterations): print(''.join(random.choices(self.chars, k=self.length))) 

Answer 3

Nomenclature

Your class is called password. A password is usually a string representing a secret. Your class, however, does not represent such a thing. It merely stores information about how a password should be generated. Hence a better name would be PasswordGenerator.

Stop writing classes

Your class has two methods, one of which is __init__. This is usually a sign, that you should not be writing a class in the first place. Try an imperative approach instead.

Separate your concerns

Most of your business logic is done in generate(). It, however does multiple things, that we can split up into different functions.

import random import string from typing import Iterator def get_pool(*, upper: bool = True, lower: bool = True, numbers: bool = False, special: bool = False) -> str: """Returns a character pool for password generation.""" pool = [] if upper: pool.append(string.ascii_uppercase) if lower: pool.append(string.ascii_lowercase) if numbers: pool.append(string.digits) if special: pool.append(string.punctuation) if pool: return ''.join(pool) raise ValueError('Pool is empty.') def generate(length: int, pool: str) -> str: """Generates a password with the given length from the given pool.""" return ''.join(random.choices(pool, k=length)) def generate_multiple(amount: int, length: int, pool: str) -> Iterator[str]: """Generates a password with the given length from the given pool.""" for _ in range(amount): yield generate(length, pool) def main(): """Showcase the above code.""" pool = get_pool(upper=False, lower=True, numbers=True, special=False) for password in generate_multiple(3, 20, pool): print(password) if __name__ == '__main__': main() 

Document your code

Your use case is a good example why code documentation is important. The user could be lead to believe, that e.g. numbers=True guarantees that the password will contain numbers, which it does not. It merely extends the password generation pool with numbers, so that the likelihood of generating a password with a number in it is > 0. However, by chance, an arbitrarily generated password might still contain no number. Those kind of behavioral quirks should be documented.