Thoughts on using isinstance

[Duncan Booth]

"abcd" <codecraig at gmail.com> wrote:

> In my code I am debating whether or not to validate the types of data
> being passed to my functions.  For example
> 
> def sayHello(self, name):
>     if not name:
>         rasie "name can't be null"
>     if not isinstance(name, str):
>         raise "name must be a string"
>     print "Hello " + name
> 
> Is the use of isinstance a "bad" way of doing things?  is it a "heavy"
> operation?  for example, if I use this in each function validate input
> will it slow things down a lot?
> 
> just curious how you might handle this type of situation (other than
> not validating at all).
> 
For a start, don't raise strings as exceptions: only use instances of 
Exception.

Now consider what your first test does: it throws an error if you pass in 
an empty string. Perhaps you do want to check for that, in which case you 
will need to test for it and throw an appropriate exception.

The first test also catches values such as None, 0 or []. Do you really 
want to throw a different exception for sayHello(0) and sayHello(1)? It 
seems a bit pointless, so the first test should just check against an empty 
string and not against other false objects which would get caught by the 
second test.

Now for the second test. It would probably be useful to say in the 
exception which type was involved, not just that it wasn't a string.
An appropriate exception for these would be something like:

  TypeError: cannot concatenate 'str' and 'int' objects

since that tells you both the types and the operation that failed. Delete 
that second test altogether and you'll get an appropriate exception instead 
of a string which hides all the information.

A good rule is if you want to hide exception information from the user do 
it when displaying the exception not when raising it. That way you can get 
at all the exception information available by changing one place in the 
code instead of having to do it everywhere.

So your modified function should look like:

def sayHello(name):
    if name=="":
        raise ValueError("name can't be blank")
    print "Hello "+name

(this is slightly different than your original in a few other ways: it will 
accept unicode strings so long as they can be encoded in ascii, and its a 
function as there isn't much point having a method which doesn't use self.)