Password Generator with injected password model

Question

I'm getting started with Python and one of the beginner projects to learn the language was a password generator (one of the dozens already in CodeReview). I have a C# and JS background so I want to make sure I'm doing things the Python way.

Technically the include_punctuation and include_digits properties could be omitted and the exclusion list would be a custom concat of strings.digits+strings.punctuation.

The reasoning behind the PasswordModel is that any component that has an API of charset and length is a valid model. That would mean we could create a MemorablePasswordModel for passwords such as "apple truck butterfly postcard" with a clean and separate model.

from random import choice
from string import digits, ascii_letters, punctuation
def filter_exclusions(items, exclusions):
 result = []
 for i in items:
 if i not in exclusions:
 result.append(i)
 return result
class PasswordModel:
 def __init__(self, length=8, include_symbols=True, include_digits=True, exclusion_list=""):
 self.exclusion_list = list(exclusion_list)
 self.length = length
 self.include_symbols = include_symbols
 self.include_digits = include_digits
 def charset(self) -> str:
 chars = [ascii_letters]
 if self.include_symbols:
 chars.append(punctuation)
 if self.include_digits:
 chars.append(digits)
 return "".join(filter_exclusions("".join(chars), self.exclusion_list))
class PasswordGenerator:
 def __init__(self, password_model=PasswordModel()):
 self.model = password_model
 def generate(self) -> str:
 charset = self.model.charset()
 return "".join(choice(charset) for _ in range(self.model.length))
# Test it out
model = PasswordModel(length=24, exclusion_list="\\/@%")
gen = PasswordGenerator()
print(gen.generate())

Questions

1. Is there a cleaner way to "include" symbols and digits, I just appended them to a list.
2. Is there anything built-in to exclude one list from another?

Answer 1

Is there a cleaner way to "include" symbols and digits, I just appended them to a list.

Cast (rather than wrap) ascii_letters etc. to a set.

Is there anything built-in to exclude one list from another?

Yes - don't use lists; use sets and apply set subtraction.

Otherwise: if you perform a cursory scan of this site for Python-based password generators you will hear some of these same points, but -

• Do not use random; use the secrets module to generate secure random numbers
• PasswordGenerator being its own separate class does not make sense; it doesn't offer anything over generate so generate can be a free function or more likely a method on PasswordModel
• charset would be better to return a set than a str
• Maybe exclusion_list is being accepted as a string due to the valid concern that mutables (lists, etc.) should not be made parameter defaults. That's fine, though a type hint exclusion_list: Iterable[str] = "" should make it clear that other iterables are acceptable, such as a set or tuple.

Answer 2

You have written the code carefully, explained your reasoning well, and asked sensible questions. That said, I would encourage you to step back and ask whether the whole thing is over-engineered. The obvious alternative is a single, simple function that has a few keyword arguments and returns the password as a string:

def password(...) -> str:
 ...

How does creating a class called PasswordModel enhance the user's ability to customize behavior? It doesn't, because it's barely more than a data-class -- in other words, it's just a bundle of function kwargs. The one exception to that claim is the charset() method, which gives the user the ability to obtain the universe of characters used in the generation process. Maybe that's useful, maybe not. But let's assume it is. Can we achieve the simplicity of a function-based implementation with the added benefit of providing one or more informational methods or attributes to users, like charset()? Yes, just use a simple class and be sure to define __str__(). Here's the idea as a minimal sketch:

class Password:
 def __init__(self, length, chars = None):
 self.length = length
 self.chars = chars or ascii_letters
 self.password = ''.join(choice(self.chars) for _ in range(self.length))
 def __str__(self):
 return self.password

If your goal is to generalize this to include not just character-based password generation but also word-based generation (eg, gluing together common words into password phrases), you might also want to rethink the terminology: elements rather than chars or charset; n rather than length; and so forth.

But, honestly, I don't care for any of these class-centric approaches. A password is a very simple thing: it's just data. As such, it should be immutable. We could mess around to try to make our Password class quasi-immutable, but things are a lot easier if we go back to a simple function. But instead of returning a string, it could return an immutable data object. For example:

from dataclasses import dataclass
# The data is simple and immutable.
@dataclass(frozen = True)
class Password:
 password: str
 elements: tuple
 def __str__(self):
 return self.password
# The algorithm is just a function, but still customizable.
def password(n, elems = None):
 elems = tuple(elems or ascii_letters)
 password = ''.join(choice(elems) for _ in range(n))
 return Password(password, elems)

• Reinderien: Your Password is a somewhat strange representation. I don't see much use in pairing up a password with the elements from which it was generated. The OP's PasswordModel class accurately represents one concern - criteria for password generation - and needn't be coupled with a totally different concern, which is a single instance of a generated password.
• FMc: @Reinderien I agree: Password.elements is not so useful, but I'm taking the implied goals of the OP's code as a given. Saying that we have a properly defined set of classes, each with their careful boundaries between concerns, is a kind of pseudo-rigor in a case this simple. The more practical and useful boundary is between the immutable data and the algorithmic behavior. If it were me, I'd just have a function returning an immutable string. But if elements is somehow useful information needed later, it can be accommodated simply enough, as illustrated.
• FMc: @Reinderien To elaborate, my prior comment doesn't adequately convey how much I do agree with your underlying point. If a person wanted to combine your point with the suggestions in my answer, the end result could be a password() function that takes immutable data for password-generation (frozen dataclass) and that returns immutable data for a password (could be str or a frozen dataclass).
• ZeroBased_IX: Thank you for the thoughtful response. It is absolutely (and intentionally) over-engineered to try and cram as much Python stuff into a simple project to learn as much as possible, however your criticism is perfectly warranted. This is code review after all! You both have given me a lot to think about going forward and I appreciate the time both of you have taken with this review.