raise None

[Steven D'Aprano]

On Thu, 31 Dec 2015 11:38 am, Chris Angelico wrote:

> On Thu, Dec 31, 2015 at 11:09 AM, Steven D'Aprano <steve at pearwood.info>
> wrote:
>> I have a lot of functions that perform the same argument checking each
>> time:
>>
>> def spam(a, b):
>>     if condition(a) or condition(b): raise TypeError
>>     if other_condition(a) or something_else(b): raise ValueError
>>     if whatever(a): raise SomethingError
>>     ...
>>
>> def eggs(a, b):
>>     if condition(a) or condition(b): raise TypeError
>>     if other_condition(a) or something_else(b): raise ValueError
>>     if whatever(a): raise SomethingError
>>     ...
>>
>>
>> Since the code is repeated, I naturally pull it out into a function:
>>
>> def _validate(a, b):
>>     if condition(a) or condition(b): raise TypeError
>>     if other_condition(a) or something_else(b): raise ValueError
>>     if whatever(a): raise SomethingError
>>
>> def spam(a, b):
>>     _validate(a, b)
>>     ...
>>
>> def eggs(a, b):
>>     _validate(a, b)
>>     ...
>>
>>
>> But when the argument checking fails, the traceback shows the error
>> occurring in _validate, not eggs or spam. (Naturally, since that is where
>> the exception is raised.) That makes the traceback more confusing than it
>> need be.
> 
> If the validation really is the same in all of them, then is it a
> problem to see the validation function in the traceback? Its purpose
> isn't simply "raise an exception", but "validate a specific set of
> inputs". That sounds like a perfectly reasonable traceback line to me
> (imagine if your validation function has a bug).

Right -- that's *exactly* why it is harmful that the _validate function
shows up in the traceback.

If _validate itself has a bug, then it will raise, and you will see the
traceback:


Traceback (most recent call last):
  File "spam", line 19, in this
  File "spam", line 29, in that
  File "spam", line 39, in other
  File "spam", line 5, in _validate
ThingyError: ...


which tells you that _validate raised an exception and therefore has a bug.
Whereas if _validate does what it is supposed to do, and is working
correctly, you will see:

Traceback (most recent call last):
  File "spam", line 19, in this
  File "spam", line 29, in that
  File "spam", line 39, in other
  File "spam", line 5, in _validate
ThingyError: ...

and the reader has to understand the internal workings of _validate
sufficiently to infer that this exception is not a bug in _validate but an
expected failure mode of other when you pass a bad argument.

Now obviously one can do that. It's often not even very hard: most bugs are
obviously bugs, and the ThingyError will surely come with a descriptive
error message like "Argument out of range" in the second case.

In the case where _validate *returns* the exception instead of raising it,
and the calling function (in this case other) raises, you see this in the
case of a bug in _validate:

Traceback (most recent call last):
  File "spam", line 19, in this
  File "spam", line 29, in that
  File "spam", line 39, in other
  File "spam", line 5, in _validate
ThingyError: ...

and this is the case of a bad argument to other:

Traceback (most recent call last):
  File "spam", line 19, in this
  File "spam", line 29, in that
  File "spam", line 39, in other
ThingyError: ...


I think this is a win for debuggability. (Is that a word?) But it's a bit
annoying to do it today, since you have to save the return result and
explicitly compare it to None. If "raise None" was a no-op, it would feel
more natural to just say raise _validate() and trust that if _validate
falls out the end and returns None, the raise will be a no-op.



-- 
Steven