On Thu, 31 Dec 2015 11:38 am, Chris Angelico wrote: > On Thu, Dec 31, 2015 at 11:09 AM, Steven D'Aprano <st...@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 -- https://mail.python.org/mailman/listinfo/python-list
