When to use assert

[Steven D'Aprano]

Anton wrote:

> I use ORM and often need to write a function that either takes an id of
> the record or already loaded model object. So I end up writing a piece of
> code like below:
> 
> def do_something(instance):
>     if isinstance(instance_or_id, int):
>         instance = Model.get(instance)
>         assert isinstance(instance, Model)
>         # Code that assumes that instance is an object of type Model
>    
> do_somthing is not a part of public library, though it is a public
> function, which can and intended to be used by other programmers; and
> assert should never happen if a client uses the function as planned.

I think you mean the assert should never fail.

That seems like a reasonable use for assert, with a proviso below. It's
behaving like a checked comment or a post-condition test: asserting that
Model.get returns a Model instance.

But, the idea of *requiring* Model.get to return a Model instance may be
inadvisable. It goes against duck-typing, and it prevents Model from making
some kinds of implementation changes that might break your post-condition
that get() always returns an instance. For example, it might return a proxy
object instead, and then your assert will fail.


> I 
> wonder if this use-case is controversial to this part:
> 
>> Many people use asserts as a quick and easy way to raise an exception if
>> an argument is given the wrong value. But this is wrong, dangerously
>> wrong, for two reasons. The first is that AssertionError is usually the
>> wrong error to give when testing function arguments. You wouldn't write
>> code like this:
>> 
>> if not isinstance(x, int):
>>     raise AssertionError("not an int")
>> 
>> you'd raise TypeError instead. "assert" raises the wrong sort of
>> exception.

No, because the nature of the exception depends on the intent of the test
and the audience who is expected to see it. In an ideal world,
AssertionError should never be seen by the end user, or the developer
calling your code (assuming that she obeys the documented requirements of
your code). A failed assert should be considered an internal error, which
the user never sees. Since the failure:

"Model.get has stopped returning Model instances"

is likely to be an internal problem ("oops, I broke the get() method, better
fix that") rather than an expected error, using assert is okay.

What would *not* be okay is something like this:


def do_something(instance_or_id):
    if isinstance(instance_or_id, int):
        instance = Model.get(instance_or_id)
    assert isinstance(instance, Model)
    # Code that assumes that instance is an object of type Model


since that fails with (for example) do_something(None): either the assert is
not checked at all, and there will be some mysterious failure deep inside
your code, or the caller will see AssertionError instead of TypeError,
violating user expectations and good design that type errors should raise
TypeError.

A better way to write this might be to have Model.get() responsible for the
error checking, and then just delegate to it:

class Model:
    def get(self, obj):
        if isinstance(obj, Model):
            return obj
        elif isinstance(obj, int):
            model = Model("do something here")
            return model
        raise TypeError('expected an int ID or a Model instance')


def do_something(instance_or_id):
    instance = Model.get(instance_or_id)
    assert isinstance(instance, Model)
    # Code that assumes that instance is an object of type Model


That means that the logic for what is acceptable as a Model is all in one
place, namely the Model.get method, and callers don't need to care about
the pre-condition "argument is a Model or an integer ID", they only need to
care about the post-condition "result of get() is a Model".



-- 
Steven