fix: don't test for runtimeType equality for object comparison

[devmil]

Status

READY

Breaking Changes

NO

Description

This PR fixes an issue introduced with 2.0.6 that int and double num values are no longer treated as equal (like 0 != 0.0)

Todos

• Tests
• Documentation
• Examples

Steps to Test or Reproduce

Have an equatable class with a num field.
Create two instances, set the num field to 0 on the first instance and to 0.0 on the second instance.
Test equality.

instance1.numField = 0;
instance2.numField = 0.0;

assert(instance1 == instance2); //breaks with 2.0.6

Impact to Remaining Code Base

Should be fine

[felangel]

I’ll take a look shortly! Sorry for the inconvenience and thanks for the PR 💙

[felangel]

This isn't quite correct. I merged #189 which should fix this specifically.

[felangel]

This should be fixed in v2.0.7 🎉

[devmil]

Thanks! But why is the runtime type check needed at all? Is there a use case that would yield a false positive in that case?

[devmil]

@felangel after thinking more about it I came to the conclusion to even strongly disagree 😁
I think the assumption here to know all the special cases and test for runtime type equality is undermining the idea of the equality operator and the "num case" was just one symptom.
There might be structures / situations in which it is desired to treat objects of different classes equal and this is expressed by overriding the equality operator.
IMHO equatable should not make any assumptions there and just use the equality operator contract to test for equality.

[devmil]

addition: I already found one case in our code base.
That special case is - for example - creating a custom File Mock that implements File and overrides the equality operator to to be comparable to a real File. This is done to be usable in a test context.
There might be better solutions for testing in that scenario but it shows that the world out there is very diverse and I think a package as central and generic as equatable needs to work in all valid conditions.

[felangel]

@devmil take the following case:

class Dog extends Equatable {
  Dog(this.name);

  final String name;

  @override
  List<Object> get props => [name];
}

class Cat extends Equatable {
  Cat(this.name);

  final String name;

  @override
  List<Object> get props => [name];
}

void main() {
  final fluffyDog = Dog(“fluffy”);
  final fluffyCat = Cat(“fluffy”);

  print(fluffyDog == fluffyCat);
}

An instance of Dog and an instance of Cat should not be considered equal even if they have the same name. Apologies for code/format I wrote this on my phone 😅.

[devmil]

@felangel

An instance of Dog and an instance of Cat should not be considered equal even if they have the same name. Apologies for code/format I wrote this on my phone 😅.

Not bad for typing that on a phone 😅

I think this case is covered already as the initial check for the runtimeType stays (

equatable/lib/src/equatable.dart

Line 48 in 834d167

runtimeType == other.runtimeType &&

and

equatable/lib/src/equatable_mixin.dart

Line 24 in 834d167

runtimeType == other.runtimeType &&

)
This check is only removed when bridging over to non-Equatable territory

So it is the responsibility of the equals operator implementation to decide if this check is needed, or not

[felangel]

@felangel

An instance of Dog and an instance of Cat should not be considered equal even if they have the same name. Apologies for code/format I wrote this on my phone 😅.

Not bad for typing that on a phone 😅

I think this case is covered already as the initial check for the runtimeType stays (

equatable/lib/src/equatable.dart

Line 48 in 834d167

runtimeType == other.runtimeType &&

and

equatable/lib/src/equatable_mixin.dart

Line 24 in 834d167

runtimeType == other.runtimeType &&

)
This check is only removed when bridging over to non-Equatable territory
So it is the responsibility of the equals operator implementation to decide if this check is needed, or not

I’ll take a closer look shortly. Feel free to reopen the PR and make sure the above test case is included 👍

[devmil]

@felangel
I'm not able to re-open this PR so I created a new one: #192
I tried to come up with some example of a custom equality logic but it's quite far-fetched. Maybe you have a better idea 😅