2

Is there ever a reason not to do this to compare two objects:

def __eq__(self, other):
    return self.__dict__ == other.__dict__

as opposed to checking each individual attribute:

def __eq__(self, other):
    return self.get_a() == other.get_a() and self.get_b() == other.get_b() and ...

Initially I had the latter, but figured the former was the cleaner solution.

darrienkennedy
  • 88
  • 7
  • "Explicit is better than implicit." – chepner Apr 20 '18 at 19:21
  • 1
    @chepner Comparing the objects' dicts makes it very explicit that _all_ of their attributes have to be equal, IMO. Much more so than manually listing half a dozen of attributes does. – Aran-Fey Apr 20 '18 at 19:22
  • Yes, but you aren't being explicit about which attributes those are. Someone might add attributes to an instance that you didn't expect. – chepner Apr 20 '18 at 19:24
  • 2
    You should really have something like `if isinstance(self, other.__class__)` before you compare anything. – user3483203 Apr 20 '18 at 19:25
  • 2
    Two instances don't necessarily need to be of the same type to be equal: `1 == 1.0`. – chepner Apr 20 '18 at 19:30

2 Answers2

4

You can be explicit and concise:

def __eq__(self, other):
    fetcher = operator.attrgetter("a", "b", "c", "d")
    try:
        return self is other or fetcher(self) == fetcher(other)
    except AttributeError:
        return False

Just comparing the __dict__ attribute (which might not exist if __slots__ is used) leaves you open to the risk that an unexpected attribute exists on the object:

class A:
    def __init__(self, a):
        self.a = a
    def __eq__(self, other):
        return self.__dict__ == other.__dict__

a1 = A(5)
a2 = A(5)
a1.b = 3
assert a1 == a2  # Fails
chepner
  • 497,756
  • 71
  • 530
  • 681
  • Maybe `_fetcher` could be an attribute of the class in order to not rebuild it every time the `__eq__` check is run. What do you think? – timgeb Apr 20 '18 at 19:35
  • 1
    Probably only if I determined that `__eq__` *needs* to be faster. If it does, I might define `_fetcher = staticmethod(attrgetter(...))`, both to emphasize that it doesn't really depend the class or any particular instance, and that it's an implementation detail of the class. – chepner Apr 20 '18 at 19:39
  • 1
    A quick test shows the code in the answer runs in 755 ns; defining `fetcher` as a static method instead of a method-local function reduces it to 650 ns, and getting rid of it altogether in favor of `(self.a, self.b, self.c) == (other.a, other.b, other.c)` is the fastest at 464 ns. – chepner Apr 20 '18 at 19:52
0

Some comments:

You should include a self is other check, otherwise, under certain conditions, the same object in memory can compare unequal to itself. Here is a demonstration. The instance-check chrisz mentioned in the comments is a good idea as well.

The dicts of self and other probably contain many more items than the attributes you are manually checking for in the second version. Therefore, the first one will be slower.

(Lastly, but not related to the question, we don't write getters and setters in Python. Access attributes directly with the dot-notation, and if something special needs to happen when getting/setting an attribute, use a property.)

timgeb
  • 76,762
  • 20
  • 123
  • 145