### 1. Review

1. It would be substantially simpler to use a *mixin* instead of a decorator.

1. The test for equality is not symmetric: you have `a == b` if `b` belongs to a subclass of `a`'s class, but not vice versa. So it's possible to have `a == b` but `b != a`, which makes no sense.

 There are two sensible things to do here: either (i) ignore the class relationship, so that two objects compare equal if their `vars` are equal, regardless of which class they belong to; or (ii) insist that classes match, so that two objects compare equal only if they belong to the same class.

1. The intention seems to be for `eqhash` objects to hash based on their instance attributes, so that instances with different attributes get different hashes. But the code doesn't work! Here are two objects with different instance attributes, but whose hashes are the same:

        >>> @eqhash
        ... class Data(SData):
        ...     pass
        >>> d = Data(1)
        >>> hash(d)
        -9223372036573986785
        >>> e = Data(2)
        >>> hash(e)
        -9223372036573986785

 And here is one object that has different hashes at different times:

        >>> d = Data(1)
        >>> hash(d)
        -9223372036573986785
        >>> vars(d).values()
        dict_values([1])
        >>> hash(d)
        -9223372036574014153

 The problem is that you are constructing a `dict_values` object and then taking its hash. But `dict_values` objects don't hash based on their contents, only on their `id` (see [Python issue 22192][1]), so the hash doesn't tell you anything about the values, only about the location in memory of the `dict_values` object.


### 2. Revised code

    class EqHash:
        """Mixin adding __eq__, __ne__, and __hash__ methods."""
        def __eq__(self, other):
            return (self is other
                    or (type(self) == type(other)
                        and vars(self) == vars(other)))

        def __ne__(self, other):
            return not (self == other)

        def __hash__(self):
            return hash(tuple(sorted(vars(self).items())))

    class Repr:
        """Mixin adding a __repr__ method."""
        def __repr__(self):
            return '{name}({values})'.format(
                name=type(self).__name__,
                values=', '.join(map(repr, vars(self).values())))


### 3. Answers to questions

1. [`functools.total_ordering`][2] is a decorator becuase it looks at the class in order to determine which ordering method was provided. See [the implementation][3], which calls `getattr(cls, op, None)`. It would be much harder to do this in a mixin.

1. I spotted the problems by thinking carefully!

 First, equality is a symmetric relation, but `isinstance` is asymmetric, so it was suspicious that one was implemented using the other.

 Second, it only makes sense to implement `__hash__` on immutable objects, but the values of a dictionary are mutable, so this was also suspicious. See the [documentation for `__hash__`][4], where it says:

 > If a class defines mutable objects and implements an `__eq__` method, it should not implement `__hash__`, since the implementation of hashable collections requires that a key’s hash value is immutable (if the object’s hash value changes, it will be in the wrong hash bucket).


  [1]: http://bugs.python.org/issue22192
  [2]: https://docs.python.org/3/library/functools.html#functools.total_ordering
  [3]: https://hg.python.org/cpython/file/tip/Lib/functools.py#l192
  [4]: https://docs.python.org/3/reference/datamodel.html#object.__hash__