testing equality of derived types

[ride4sun]

Describe the bug I have two types:

class GenericDevice extends Equatable {
  final String name;
  final String id;

  GenericDevice({@required this.name, @required this.id});

  @override
  bool get stringify => true;

  // named constructor
  GenericDevice.fromJson(Map<String, dynamic> json)
      : name = json['name'],
        id = json['id'];

  // method
  Map<String, dynamic> toJson() {
    return {
      'name': name,
      'id': id,
    };
  }

  @override
  String toString() {
    return 'GenericDevice{name: $name, id: $id}';
  }

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

and

class BleDevice<T> extends GenericDevice with Disposable {
  BleDevice({@required this.device, @required String name, @required String id})
      : super(name: name, id: id);

  final T device;
}

I Wrote a test like this:

  test(
    'Generic Device Test - Equatable test',
    () {
      var device1 = GenericDevice(id: '1234', name: 'deviceName');
      var device2 = BleDevice<int>(id: '1234', name: 'deviceName', device: 6);
      var device3 = BleDevice<int>(id: '12345', name: 'deviceName', device: 6);
      print(device1);
      print(device2);
      print(device3);

      expect(device1 == device2, equals(true));
      expect(device1, isNot(equals(device3)));
    },
  );

I expected the test to pass but because because Equality is looking into the runtime type its failed

  @override
  bool operator ==(Object other) {
    return identical(this, other) ||
        other is EquatableMixin &&
            runtimeType == other.runtimeType &&
            equals(props, other.props);
  }

Any suggestions ideas - is that really how it should be?

Makes me wonder if that could not be enough:

@override
  bool operator ==(Object other) =>
      identical(this, other) ||
      other is Equatable && equals(props, other.props);

Is the dart analyzer not catching that I compare not comparable types?

[felangel]

Hi @ride4sun 👋 Thanks for opening an issue!

I believe this is working as expected because a GenericDevice is technically not equal to a BleDevice. If you wanted to make that true you could override operator== on your BleDevice class and include GenericDevice like:

import 'package:collection/collection.dart';
import 'package:equatable/equatable.dart';

abstract class Animal extends Equatable {
  const Animal({required this.name});
  final String name;

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

mixin AnimalEqualityMixin on Animal {
  static const _equality = DeepCollectionEquality();

  @override
  bool operator ==(Object other) {
    return other is Animal && _equality.equals(props, other.props);
  }
}

class Cat extends Animal with AnimalEqualityMixin {
  const Cat({required String name}) : super(name: name);
}

class Dog extends Animal with AnimalEqualityMixin {
  const Dog({required String name}) : super(name: name);
}

void main() {
  final cat = Cat(name: 'Taco');
  final dog = Dog(name: 'Taco');

  print(cat == dog); // true
}

I don't think this is within the scope of Equatable because I normally wouldn't consider an instance of a Dog and an instance of a Cat to be equal even if they have the same name, for example.

Hope that helps 👍

[mrgnhnt96]

Here is a possible solution for this #133

[SupposedlySam]

Here is a possible solution for this #133

Essentially Morgan and I created an optional property you can override called derived. If you're just extending or mixing in equatable then there's no need to do anything different. By default the derived class uses the runtimeType within the Set for derived.

However, if you're creating a "derived" class, then you provide the type you want your class to be compared against.

In your example @ride4sun, all you'd need to do is add an override to your BleDevice class

class BleDevice<T> extends GenericDevice with Disposable {
  BleDevice({@required this.device, @required String name, @required String id})
      : super(name: name, id: id);

  final T device;

  @override
  Set<Type> get derived => {GenericDevice};
}

Then when you run your tests they will work as expected.

You can give this a try by switching over to branch in your pubspec.yaml file

  equatable:
    git:
      url: 'https://github.com/mrgnhnt96/equatable.git'
      ref: derived

if it makes it easier you can add it under your top level dependency_overrides: section inside the pubspec.yaml, or create a new section by that name.

[AlexMcConnell]

@felangel

If you assume that all inheritance follows a hierarchical pattern similar to animals, then sure, checking equality between types makes little sense. But that's only a fraction of usages for inheritance. Personally, I'm creating test objects that inherit from the real objects, and not being able to compare them with object that are created by the app defeats the purpose.

[felangel]

@felangel

If you assume that all inheritance follows a hierarchical pattern similar to animals, then sure, checking equality between types makes little sense. But that's only a fraction of usages for inheritance. Personally, I'm creating test objects that inherit from the real objects, and not being able to compare them with object that are created by the app defeats the purpose.

Can you provide a link to a DartPad/Gist/Repository that illustrates what you’re trying to achieve? Thanks!

[FMorschel]

Here is a possible solution for this #133

Essentially Morgan and I created an optional property you can override called derived. If you're just extending or mixing in equatable then there's no need to do anything different. By default the derived class uses the runtimeType within the Set for derived.

However, if you're creating a "derived" class, then you provide the type you want your class to be compared against.

In your example @ride4sun, all you'd need to do is add an override to your BleDevice class

class BleDevice<T> extends GenericDevice with Disposable {
  BleDevice({@required this.device, @required String name, @required String id})
      : super(name: name, id: id);

  final T device;

  @override
  Set<Type> get derived => {GenericDevice};
}

@SupposedlySam, could you try and help me? I've created #147 for a better explanation for my case, but basically, my question is: When your derived class is an Enum, where overrides on == related methods don't work at all. What can I do? I've solved my case on the end of my issue, but I still believe this should probably be implemented by the equatable package itself.

[SupposedlySam]

@FMorschel , maybe you could create an extension method on your enum that takes in the second enum type and compares them to return true or false?

MyEnum.equals(MyOtherEnum); // true

It wouldn't be generic but maybe you can work with that?

[FMorschel]

As I've said on #147

As commented by @Levi-Lesches here the answer to my problem was to override my operator ==:

  @override
  // ignore: hash_and_equals, already implemented by EquatableMixin
  bool operator ==(Object other) {
    return (super == other) ||
        ((other is Class) &&
            (aValue == other.aValue) &&
            (bValue == other.bValue));
  }

This is done in my original class that my Enum implements.

This, I believe is how this package should solve the equality of instances, using is Type not runtimeType. That's what I meant.

This is fair because my enum constants can be considered as an instance of my class, but not the other way around because my class is not even an Enum.

So even if I tried using your suggestion above, my problem wouldn't be solved. I know creating a new method like the equals you mentioned could work, but that's not quite what I was aiming for.

[michaelsoliman1]

I think this suggestion can work!

I think it would be a better idea to add a configuration (like we have for stringify) to include runtimeType in equality instead of "derived".

we can have both options, but for me i want this to be a global rule because all of the derived classes are the same as their parents

Edit

although this will do what we need, but it will also create other equality issues (e.g generated code with Freezed)

[felangel]

Going to close this for now since it's quite old and it seems to be out of scope for this package imo. Happy to revisit the conversation if others feel strongly though.