Update: Alternative approaches
It was pointed out to me in a comment that the class I'd made contains only one non-__init__ method, which is often a sign of an unnecessary class. I agree that that is often the case, and while I would argue that it might be justified in this particular case, other approaches are worth looking at
A single function
Often, a single-method class can be simplified to a single function, and if some of its arguments need to be saved functools.partial can be used. A bit like:
def generate_password(config_file: str, strength: str) -> str:
with open(config_path) as config_file:
return generate_gibberish(yaml.safe_load(config_file)[strength])
# Example usage
cfg = functools.partial(generate_password, config_file_path)
for _ in range(100):
print(cfg("medium"))
Unfortunately, this runs into the same issue as the original code, in that it needs to re-open and re-parse the config file each time it generates a password. This is a nice helper function, but shouldn't be the entire API on its own
Configuration as message
A natural way around that issue would be to instead have two separate functions - one to load the configuration, another which accepts a configuration and a strength. That might look like this:
def load_config(config_path: str) -> dict[str, int]:
with open(config_path) as config_file:
return yaml.safe_load(config_file)
def generate_password(config: dict[str, int], strength: str) -> str:
return generate_gibberish(config[level])
# Example usage
cfg = load_config(config_file_path)
for _ in range(100):
print(generate_password(cfg, "medium"))
Semantically, this is a very nice approach. It does have one drawback however - it makes the structure of the config into part of the module's interface. Right now we promise that the functions will only produce and consume ints. If we want to change that and add a more complex kind of security level some day, that would require an annoying amount of care to not break the API. I consider that kind of restriction enough of a drawback that I would favor another option
Configuration as callable
We can keep similar semantics without tying our public interface to our internals. Making the configuration into functions can keep the internals flexible by not exposing them:
def load_config(config_path: str) -> Callable[[str], str]:
with open(config_path) as config_file:
lengths = yaml.safe_load(config_file)
def generate_password(strength: str) -> str:
return generate_gibberish(lengths[str])
return generate_password
# Example usage
cfg = load_config(config_file_path)
for _ in range(100):
print(cfg("medium"))
This keeps our internals flexible, maintains easy-to-understand semantics, and generally avoids the drawbacks of the earlier approaches. Though admittedly, we now have to start worrying about how to turn a configuration into YAML
This approach is similar to the original - if the original one had used __call__ instead of a named method, it would've had almost exactly this interface. By not using a class it's a bit simpler, though if we want to extend it further (say, by adding the ability to check if "weak" in cfg), the class-based original may be easier to work with. Or is there's another way we can get that?
Configuration as name-generator mapping
There's no reason using functions has to mean we can't also use a dict, right?
def load_config(config_path: str) -> dict[str, Callable[[], str]]:
with open(config_path) as config_file:
lengths = yaml.safe_load(config_file)
return {strength: functools.partial(generate_gibberish, length) for (strength, length) in lengths.items()}
# Example usage
cfg = load_config(config_file_path)
for _ in range(100):
print(cfg["medium"]())
I do like this approach. It's easy to implement and gets some useful functionality by default that the original doesn't (like being able to check if "weak" in cfg, or get a list of strength levels with cfg.keys()). Those are pretty strong argument in favor of this approach
But while load_config(config_file_path)["weak"]() is easy to understand, I have to say I don't much like the aesthetics. That's less important than the extra functionality of course, but then again there's nothing stopping you from adding that functionality to a class-based approach if you want.
Is there a conclusion?
The basic two-function approach ties your API to your internals, but isn't bad otherwise I guess. I'd probably recommend the dict-of-functions approach, it's simple, powerful and intuitive. A class-based approach works pretty well still (and makes for a less ugly interface)