-
Notifications
You must be signed in to change notification settings - Fork 95
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
Implement the Hashable interface on Container classes ? #65
Comments
I like this, but I'm not sure how I would handle the hash value. I would prefer to implement an |
Hi @rtheunissen any plan about this Just see that |
Ok, this is not a BC break feature... any chance to get it backported to |
Doesn't need internal refactoring, I'm really just lumping everything into 2.0 because it's my active branch. Happy to implement in 1.x now and just merge into 2.0. |
See #67 for status on If we did implement We either have to use and support An example of a consistency I want to avoid at all costs: $a = new Ds\Set([1, 2, 3]);
$b = new Ds\Set(["1", "2", "3"]);
$a == $b; // true, because of loose property comparison.
$a === $b; // false, because they are not the same instance.
$a->equals($b); // false, because the sets don't contain the same values. The inconsistency is between the Like atoum/atoum#671 mentions, even if we implement |
As mentioned on another ticket it might be good to extract the The issues with |
Usually an interface Identifiable {
function equals($other): bool;
function hash(): string;
}
trait IdentifiableTrait {
public function equals($other): bool {
return $other instanceof $this && $other->hash() === $this->hash();
}
public function hash(): string {
return \spl_object_hash();
}
}
final class SomeEntity implements Identifiable {
use IdentifiableTrait;
private $id;
public function hash(): string {
return \md5(__CLASS__ . $this->id, \false);
}
}
final class SomeValueObject implements Identifiable {
use IdentifiableTrait;
private $some_prop;
public function hash(): string {
return \md5(\var_export($this, \true), \false);
}
} This might be a way how you could implement it. |
I'm not sure where you get that idea but that is fundamentally broken. The whole reason Edit to expand on this a bit: If |
Can you directly link me to the explanation? I see some type-juggling issues but not where two objects which are of the same type, are |
@morrisonlevi there you go: #67 (comment) |
Floating point values are not objects, do not implement Hashable, and thus do not have an |
Not in PHP, that is true. What about two I’m only trying to make clear that partial equality and ordering is something that is true for some types. The constraint you are laying out is normal for the keys of hash map implementations (if x == y then x.hash == y.hash) because they are broken if that does not hold. |
I think the decision here is that only Hashable types should be supported by hash-based structures, and the contract assumes that if x == y then x.hash == y.hash. Containers that are mutable will not implement Hashable. You would need to convert to an immutable type first. Hashable and Equatable should be separated. |
Hi,
is there a reason to not implement the
Hashable
interface on the "container" classes? I think that it will be very useful in order to compare containers.It seems that the current implementation forces users to loop over collections to compare them. I know that if this comparison is implemented as a method, it will work the same way, but I think is better to implement the comparison one single time rather than reimplement it again and again in "userland".
The text was updated successfully, but these errors were encountered: