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

• AncientSwordRage: You can store various thinks in a dict, then access them on a the key. I've not pythoned for a long time, so I'm not confident enough to write a whole answer.
• Dschoni: Why not use if-else instead of if-elif? What happens with unexpected input?
• BCdotWEB: The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?.

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.

• some_user_3: my program has changed somewhat since I asked this question but do you think the same rules apply? GitHub: github.com/lunAr-creator/pw-gen/blob/main/pw_gen/__init__.py
• Richard Neumann: If your program changed (significally), and you want to have it reviewed, you're free to post a new follow-up question.
• some_user_3: ok. ill post a new-follow up question
• some_user_3: link to new question: codereview.stackexchange.com/questions/257185/…