Idea how to clean up the parameter list of validate_email[_or_fail]

[reinhard-mueller]

With the growing number of checks that can be switched on and off, the parameter list has become somewhat long and cluttered, switches are named use_*, check_*, or skip_*, and also some have to be set to True to enable a test, some to True to disable the test.

I had the idea of replacing all these parameters with a single one:

First, we define a constant for each of the possible checks:

CHECK_FORMAT = 1
CHECK_BLACKLIST = 2
CHECK_MX = 3
CHECK_SMTP = 4
CHECK_SMTP_COMMUNICATION = 5
CHECK_SMTP_TEMPORARY = 6

and we define the default set of checks as a Python set:

CHECK_DEFAULT = {CHECK_FORMAT, CHECK_BLACKLIST, CHECK_MX, CHECK_SMTP}

Then, we can change validate_email_or_fail to accept the parameter checks: Set = CHECK_DEFAULT.

Downside: this would either break compatibility with previous versions, or we'd have to accept both options for a while, adding a deprecation warning when the old parameters are used.

What do you think?

[karolyi]

I think you have a lot of free time on your hands :) Good!

Putting the options into a set is not a good idea, as the order of the values within are not preserved in python3.7 and below (I think), but a dict for the options might be a good candidate here. Not much different from a picked up kwargs argument though, maybe a bit more organized.

The unification of the argument names is a good idea in general imo, just to make the API a bit more straightforward.

That said, it will be a major module version change, as it will contain breaking changes: i.e. 0.2.x -> 1.0.x. I'm okay with that tough, but it has to be done this way so the users know that it's a somewhat bigger change.

[reinhard-mueller]

It's not exactly that I'd have too much free time ;-) but the library is an essential component in some of my projects.

What would you think about keeping the old parameters functional but with a deprecation warning?

[karolyi]

Ah I think I missed the point that the values for the variables are unique integers and thus the order doesn't matter. In that case, using a set is okay, but everything else I added is still valid.

[reinhard-mueller]

Now as we've decided to not have parameters about whether or not to raise exceptions, I think it's okay to keep separate parameters for the 4 checks that can be enabled or disabled.

However, I think with the major release, the parameter list could still be cleaned up:

• use_blacklist -> check_blacklist (for consistency)
• skip_smtp -> check_smtp and invert logic (for consistency)
• change order: email_address, check_regex, check_blacklist, check_mx, dns_timeout, check_smtp, smtp_timeout, helo_host, from_address, debug
• Maybe prefix helo_host, from_address, and debug with smtp_ to make clear these parameters (only) refer to the SMTP check?
• Not sure but maybe check_format would be a better name than check_regex and check_dns instead of check_mx?
• Maybe change the public functions into all keyword-only parameters, so we are free in future to add new parameters wherever they fit best?
• Maybe explicitly remove functions like mx_check from the public interface, by renaming them to e.g. _mx_check?

All of the above are just ideas, but as it might probably be quite some time until the next change to clean up the public interface, it might be worth considering what can be done now.

[karolyi]

All of these are good ideas good to go by me.

I've also added initial tests to test out the new _SMTPChecker class, you can continue with further unittests on it. You get the idea as to how to mock class elements now.

Can you make a PR against the https://github.com/karolyi/py3-validate-email/commits/pull-55 branch with your proposed changes?