Is it ok to type check a boolean argument?

[Terry Reedy]

Adal Chiriliuc wrote:

> Me and my colleagues are having an discussion about the best way to
> code a function (more Pythonic).
> 
> Here is the offending function:
> 
> def find(field, order):
> ....if not isinstance(order, bool):
> ........raise ValueError("order must be a bool")
> ....order_by = "asc" if order else "desc"
> ....return _find(field + "+" + order_by)

My opinions:
1. 'order' should be 'a' (the default) or 'd'. True and False by 
themselves are meaningless.  Then the check, if needed, is "if order not 
in 'ad':".
'up' and 'down' are possible too.
2. Consider renaming _find as find, with two params and do the parameter 
check there.  Except when one is calling a function recursively 
(repeatedly) with known-good params, wrapping a function with a simple 
arg check seems like noise to me.

> We are not sure what's the best practice here. Should we or should we
> not check the type of the "order" variable, which should be a bool?

I say it should not be a bool.  The below illustrates another reason why 
this is the wrong type.

> In one of our unit-tests we passed the "invalid_order" string as the
> order argument value. No exception was raised, since the string was
> evaluated as being True.

If you make 'order' a string, then a bad string input should raise an 
exception somewhere.  I suspect _find could be written to do this if it 
does not already.

> We know about "Don't look before we jump", but we are not sure how it
> applies in this case, since we don't get any exception when passing an
> invalid type argument.

> This function is not visible to our clients, only internally in our
> project. It's part of the public interface of a sub-system, so we are
> not sure either if the fact that it returns an invalid result for a
> badly type argument it's an API specification break or not.
> 
> The pro argument was that if a new programmer comes and passes a
> wrongly typed argument, he will get a silent failure.

That is bad, but in this case, I see the problem as a slight mis-design.

> The cons argument was that there are many functions which could
> silently fail in this mode, especially those having bool arguments.

Which are rather rare, I think.  In the 3.0 builtin functions, sorted's 
'reverse' param is the only one.  For back compatibility, it actually 
accepts ints.

> Should we in general check when doing unit-testing that the methods
> behave correctly when passed arguments of the wrong type?

As in 'raise an exception', I think at least one test is good.

Terry Jan Reedy