Missing equalTo() method #4
Comments
|
Yep that would definitely make sense! |
|
See also #8 you can use if ($enum == MyEnum::FOO()) {
}So in the end I'm not sure a |
|
@mnapoli I was about to write the same yesterday. |
|
OK closing for now. |
|
+1 for a 'equalsTo' method. public function equalsTo($enum)
{
return $this == $enum;
}$enum1 = Enum::KEY_ONE();
$enum2 = Enum::KEY_TWO();
$enum1->equalsTo($enum2); (false)
$enum1->equalsTo($enum1); (true) |
|
I was looking for this functionality last night. Added PR #39 + a test. |
|
#39 has been merged, I'll give it a day or two for everyone to review (given how used the library is) and then I'll tag Thanks @jeremykendall |
|
@mnapoli You're welcome! Thanks for letting me contribute! |
|
@mnapoli @jeremykendall My team and I use this library as a base for our own enum library (a wrapper, you can call it) and we tend to add features that seems necessary for us and maybe doesn't warrant the inclusion to The need for The beauty there is "equals defers to ==". That means, if you don't override it, both will work the same way. Also, Java internals use Back to PHP, that's not how it works. PHP internals will almost always use final class Color extends \MyCLabs\Enum\Enum {
const RED = 'red';
const BLUE = 'blue';
}
$red = COLOR::RED();
$blue = COLOR::BLUE();
$anotherRed = COLOR::RED();
// Equality check using == // expected
var_dump($red == $red); // true
var_dump($red == $blue); // false
var_dump($red == $anotherRed); // true
// Equality check using equals() // expected
var_dump($red->equals($red)); // true
var_dump($red->equals($blue)); // false
var_dump($red->equals($anotherRed)); // trueAll good, there is no blessed equality checking method, user uses whichever preferred. Then, user goes and overwrites the There is a simple fix which would make the design of that feature better - declare the Maybe I'm being picky now, but that's what I expect from a good enum library in php if it introduces a third way of comparing objects. Not only enum library, the same goes for value objects. btw. another way of solving that is going full-Java: abstract class JavaLikeEnum extends \MyCLabs\Enum\Enum {
final public function equals(\MyCLabs\Enum\Enum $outsider)
{
return $this->hashCode() == $outsider->hashCode();
}
protected function hashCode() {
return $this->getValue();
}
}if you want to change what |
|
I'm always game for Awesome input, @mirfilip! On Wednesday, October 5, 2016, Matthieu Napoli notifications@github.com
Sent from Gmail Mobile |
|
@mirfilip - Good use of Final. Just shared it with my dev team as an example - thanks. |
|
Ok let's do it then, @jeremykendall could you add it? |
|
@jeremykendall When you modify the PR, would you mind adding more tests? As it stands, two cases are tested:
The missing case is: |
|
Thanks, everyone! I'd love to add |
|
I couldn't resist. Just submitted PR #40. |
|
All is good now, we can close this I'll tag a release |
|
1.5.0 🎉 |
|
Gonna have to say -1 on the
Final would be ok if this was a new major version, but as it is it's a breaking change. |
Ugh. I hate that, and I know it's super frustrating.
Although your custom
That said, I'm interested to hear more about your |
That's the issue, it wasn't added in a backwards compatible manner. There's no difference in functionality, it breaks because of the Don't get me wrong, I welcome the addition of the method to this package, it just shouldn't be |
|
@sebastiandedeyne I may be missing something here. A Major version is required when the change breaks compatibility in the source lib, e.g. an API changes. I'm not sure that adding a new method (with or without final) that may clash with unknown downstream code constitutes a breaking change. After all, that way, we may never get enhancements into the code base. But be interesting to hear other views too. |
I have the same question, final is supposed to prevent overriding the method so I'm curious why (if we exclude the fact that you already defined it before) you would need to override it? |
That's my take on the semver concern (emphasis added). |
|
I think you guys are right, semver doesn't seem to consider this a breaking change. I still don't really agree with |
|
@sebastiandedeyne I feel your pain, but this is not a BC break. If you point composer to such a loose version constraint, that's what you get. BC break from semver definition treats about functionality that has been added to a library and later is being changed, so that people depending on it encounter a functionality breakage. There was no If you think about it, it makes perfect sense. I can take a well known framework (say Symfony), pick a popular class, extend it and add all the popular function names I could think of (for discussion sake, let's say it makes sense). Then, at some point, Symfony people introduce the same named function but with different definition (final, different number of arguments, different typehinting, you name it). It's not reasonable to say they would be breaking any contract of Symfony, right? Anyway, even in your definition EDIT: I just realized I had missed @chippyash post that clarifies it all. Disregard my long dispute :) EDIT 2: Just my overview of the addition of |
|
Makes sense, fair enough :) |
|
Adding On 10 Oct 2016 18:06, "Sebastian De Deyne" notifications@github.com wrote:
|
|
If adding final to an existing API method, definitely, but what about On Monday, October 10, 2016, Marco Pivetta notifications@github.com wrote:
Sent from Gmail Mobile |
|
Yeah, good point, new method is surely not a breakage, though the On 11 Oct 2016 02:44, "Jeremy Kendall" notifications@github.com wrote:
|
Sometimes i want to compare enums for equaility, currnetly i have to cast to
(string)to make it work. This could be something to add as a default method.The text was updated successfully, but these errors were encountered: