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 for Equality #172

Closed
designermonkey opened this issue May 13, 2021 · 4 comments
Closed

Testing for Equality #172

designermonkey opened this issue May 13, 2021 · 4 comments

Comments

@designermonkey
Copy link

I would like to posit an idea based on this statement from the docs in Map:

Keys of type object are supported. If an object implements Ds\Hashable, equality will be determined by the object's equals function. If an object does not implement Ds\Hashable, objects must be references to the same instance to be considered equal.

Would it be possible to test for any equals function and at least try it? What's my use case I hear you ask...

I use MyClabs\Enum\Enum a lot, at least I did until I started using DS a lot more. Now I have had to re-implement a library for Enum because I need it for keys in Maps, and I can't reliably use MyClabs\Enum\Enum as it doesn't implement Hashable.

It's ok I suppose to use my implementation, but I would rather not have to so that I can take easy advantage of updates and fixes.

I'm copying in @benjaminbertin, @valentin-claras and @mnapoli as the MyClabs team, as maybe there may be a way they'd consider supporting Ds somehow.

@rtheunissen
Copy link
Member

It's ok I suppose to use my implementation, but I would rather not have to so that I can take easy advantage of updates and fixes.

Would an extension of MyClabs\Enum\Enum which implements Hashable not also benefit from updates and fixes upstream?

@designermonkey
Copy link
Author

Sadly the equals method in that class is final.

@rtheunissen
Copy link
Member

It is unfortunate that we do have a base equals method. I think checking for an exists function and calling it is a reasonable idea, can't see why not.

@enumag
Copy link

enumag commented May 17, 2021

This works just fine for me:

<?php declare(strict_types = 1);

namespace App\Library\Enum;

use Ds\Hashable;
use MyCLabs\Enum\Enum as BaseEnum;

/**
 * @psalm-immutable
 * @extends BaseEnum<string>
 */
abstract class Enum extends BaseEnum implements Hashable
{
	public function hash(): string
	{
		return $this->getValue();
	}
}

The point is you don't need to override the equals method since the base Enum class already implements it and it is compatible with Hashable.

That said I do agree that being able to provide a custom equality function could be beneficial in some cases. It's not necessary for MyCLabs enums though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants