List comprehension for testing **params

[Steven D'Aprano]

On Sun, 11 Nov 2012 23:24:14 +0100, Cantabile wrote:

> Hi,
> I'm writing a small mail library for my own use, and at the time I'm
> testing parameters like this:
> 
> class Mail(object):
>      def __init__(self, smtp, login, **params)
>          blah
>          blah
>          required = ['Subject', 'From', 'To', 'msg'] for i in required:
>              if not i in params.keys():
>              print "Error, \'%s\' argument is missing" %i exit(1)
>      ...

Eeek! Don't do that. Seriously dude, that is Evil. Why should a simple 
missing argument to a class *kill the entire application*????

How horrible would it be if you called "chr()" and instead of raising a 
TypeError, Python shut down? Don't do that. Raise a proper exception. 
Then you have the choice to either catch the exception and continue, or 
let it propagate as appropriate.


But you shouldn't even be manually checking for required arguments. Just 
use real arguments:

    def __init__(self, smpt, login, Subject, From, To, msg, **params):
        blah 


See how easy it is? Problem solved. If you don't provide an argument, 
Python gives you an exception for free. Best of all, you can use 
inspection to see what arguments are required, and help(Mail) shows you 
what arguments are needed.

You can still use **md as below, and Python will automatically do 
everything you're manually trying to do (and failing). Don't fight the 
language, even when you win, you lose.


> md = {'Subject' : 'Test', 'From' :'Me', 'To' :['You', 'Them'], 'msg'
> :my.txt"}
> 
> m = Mail('smtp.myprovider.com', ["mylogin", "mypasswd"], **md)

Conventionally, a tuple ("mylogin", "mypasswd") would express the intent 
better than a list.


> I'd like to do something like that instead of the 'for' loop in
> __init__:
> 
> assert[key for key in required if key in params.keys()]
> 
> but it doesn't work. It doesn't find anythin wrong if remove, say msg,
> from **md. I thought it should because I believed that this list
> comprehension would check that every keyword in required would have a
> match in params.keys.

No, you have three problems there.

The first problem is that only the empty list is falsey:

assert []  # this raises an exception
assert ["Subject", "From"]  # this doesn't, even though "To" and "msg" 
are missing.

You could say:

assert all([key in params for key in required])

but that leaves you with the next two problems:

2) Fixing the assert still leaves you with the wrong exception. You 
wouldn't raise a ZeroDivisionError, or a UnicodeDecodeError, or an IOError 
would you? No of course not. So why are you suggesting raising an 
AssertionError? That is completely inappropriate, the right exception to 
use is TypeError. Don't be lazy and treat assert as a quick and easy way 
to get exceptions for free.

3) Asserts are not guaranteed to run. Never, never, never use assert to 
test function arguments supplied by the caller, because you have no 
control over whether or not the asserts will even be called.

The whole point of assertions is that the caller can disable them for 
speed. Asserts should be used for testing your code's internal logic, not 
user-provided arguments. Or for testing things which "can't possibly 
fail". But of course, since what can't go wrong does, you sometimes still 
want to test things.

If you've ever written something like:

if condition:
    process()
else:
    # this cannot happen, but just in case!
    print "something unexpected went wrong!"


that's a perfect candidate for an assertion:

assert condition, "something unexpected went wrong!"
process()



-- 
Steven