Fix for search() is not type safe. Fixes #13

[mirfilip]

@mnapoli please have a look

[mnapoli]

Sorry for the delay :)

[drealecs]

This actually brought backward incompatibilities. If you have a enum that contains constant X = 1; It was possible to use '1' (string) in place of 1 (integer) for instantiating it.

[mirfilip]

@drealecs Both this issue and #9 were considered a bug by @mnapoli. Your use case relies on buggy behaviour when instiantiating enums. If something is considered a bug, fixing it does not break BC by definition.

Anyway, I believe you address #9 rather than #14 with your concerns.

[drealecs]

Ok, it was considered a bug. It's good that this is fixed so it is caught during development now and not later.
What I said is that someone might upgrade libraries and might test it lightly because it's only a patch version increment. I did it unfortunately and I expect someone else to have done it also :)
But I'm writing here just to discuss this and maybe help and all of us learn how it is the best way to version the software :). I'm sorry I didn't said it earlier.
Thanks, great patch.

[mirfilip]

@drealecs yeah, I totally get your point :) According to semver, version number PATCH is version when you make backwards-compatible bug fixes.. True, it wasn't backward compatible and this could affect people that had jumped on 1.3.0 and then got burned by 1.3.2 without a notice. So basically:

1. I was wrong in my previous comment ;)
2. @mnapoli probably did a patch release to avoid bumping major version.

[mnapoli]

Hey guys, sorry for the late response I was on holidays without internet access.

I indeed considered this a bug so in theory the patch release makes sense, but I definitely agree that I should have been more careful with that (somehow), so my apologies :/

Do you see any way we could have non-strict comparison work for values that make sense? E.g. '1' (string) should match 1 (int) but true (bool) shouldn't. And for example '' should definitely not match 0… I don't really see how it could be done.

[mirfilip]

@mnapoli as the case of matching values you describe differs from how php treats truthy and falsy values (and casting between them), the only way it could work is with custom mapping ('1' matches 1 etc.).

The problem with any mapping, however, is that there can always be a case that seems to make sense for library user. If you intend to add some cases (like the one above) just to cover BC, I don't think it's viable.

Consts values, by design, should work as unique identifiers of enum. The problem is that it's clumsy for base Enum class to validate consts values duplication on construction.

As the bugfix is already released, you can have two groups of users: ones that already rely on hard typing and the ones, like @drealecs, who are affected by it.

Having considered both, I think having "hard typing" still saves users from many more fuckups.

BTW. We could, in theory, use Reflection at __construct to gather all declared values and make some pre-validation of uniqness but that would probably be an overkill. Maybe good doc example instead ?

[mnapoli]

Yeah I agree with you it's not inline with PHP's behavior (it would be a mix in-between weak and strict comparison), so not really possible.

I definitely like strict too, the only thing I'm wary of is imagine that (simplified) situation:

class State extends Enum
{
    const OPEN = 1;
}

$value = new State($_GET['state']);

$_GET['state'] will be a string ('1'), so people might get caught by that.

But I guess it's easy both to understand the problem (the exception will be explicit) and to fix it… So we should probably not try to cover those use cases and stay strict.