Is this use of isinstance pythonic/"good"?

Question

A side effect of this question is that I was lead to this post, which states:

Whenever isinstance is used, control flow forks; one type of object goes down one code path, and other types of object go down the other --- even if they implement the same interface!

and suggests that this is a bad thing.

However, I've used code like this before, in what I thought was an OO way. Something like the following:

class MyTime(object):
    def __init__(self, h=0, m=0, s=0):
        self.h = 0
        self.m = 0
        self.s = 0
    def __iadd__(self, other):
        if isinstance(other, MyTime):
            self.h += other.h
            self.m += other.m
            self.s += other.s
        elif isinstance(other, int):
            self.h += other/3600
            other %= 3600
            self.m += other/60
            other %= 60
            self.s += other
        else:
            raise TypeError('Addition not supported for ' + type(other).__name__)

So my question:

Is this use of isinstance "pythonic" and "good" OOP?

Answer 1

Not in general. An object's interface should define its behavior. In your example above, it would be better if other used a consistent interface:

def __iadd__(self, other):
    self.h += other.h
    self.m += other.m
    self.s += other.s

Even though this looks like it is less functional, conceptually it is much cleaner. Now you leave it to the language to throw an exception if other does not match the interface. You can solve the problem of adding int times by - for example - creating a MyTime "constructor" using the integer's "interface". This keeps the code cleaner and leaves fewer surprises for the next guy.

Others may disagree, but I feel there may be a place for isinstance if you are using reflection in special cases such as when implementing a plugin architecture.

Answer 2

To elaborate further on the comment I made under Justin's answer, I would keep his code for __iadd__ (i.e., so MyTime objects can only be added to other MyTime objects) and rewrite __init__ in this way:

def __init__(self, **params):
    if params.get('sec'):
        t = params['sec']
        self.h = t/3600
        t %= 3600
        self.m = t/60
        t %= 60
        self.s = t
    elif params.get('time'):
        t = params['time']
        self.h = t.h
        self.m = t.m
        self.s = t.s
    else:
        if params:
            raise TypeError("__init__() got unexpected keyword argument '%s'" % params.keys()[0])
        else:
            raise TypeError("__init__() expected keyword argument 'sec' or 'time'")

# example usage
t1 = MyTime(sec=30)
t2 = MyTime(sec=60)
t2 += t1 
t3 = MyTime(time=t1)

I just tried to pick short keyword arguments, but you may want to get more descriptive than I did.