Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testing equality of derived types #131

Open
ride4sun opened this issue Dec 28, 2021 · 9 comments
Open

testing equality of derived types #131

ride4sun opened this issue Dec 28, 2021 · 9 comments
Assignees
Labels
question Further information is requested waiting for response Waiting for follow up

Comments

@ride4sun
Copy link

ride4sun commented Dec 28, 2021

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
Copy link
Owner

felangel commented Jan 28, 2022

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 👍

@felangel felangel self-assigned this Jan 28, 2022
@felangel felangel added question Further information is requested waiting for response Waiting for follow up labels Jan 28, 2022
@mrgnhnt96
Copy link

Here is a possible solution for this #133

@SupposedlySam
Copy link

SupposedlySam commented Jan 28, 2022

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
Copy link

@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
Copy link
Owner

@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
Copy link

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
Copy link

@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
Copy link

FMorschel commented Aug 22, 2022

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
Copy link

michaelsoliman1 commented Dec 4, 2022

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested waiting for response Waiting for follow up
Projects
None yet
Development

No branches or pull requests

7 participants