Setting Enum values by `static::$map` leads to issues on creating instances and equality

[spatie]

Setting Enum values by `static::$map` leads to issues on creating instances and equality

[eclipxe13]

Its is project governance that should decide:

• what to do about "any type" of value defined inside static::$map or only "stringables".
  Current state is "any type".

• compare values as equality == or identity ===
  Current state is mixed, __constructor() is using in_array without strict comparison, equals uses strict comparison.

I'm just pointing out the issues I found, reporting findings is a way to contribute.

[brendt]

Thanks @eclipxe13 for the thorough report!

First of all I would like to ask @brendt if the bug is a real bug or if it's intended to allow only string values.

I'd say it's an oversight. However, I believe the way we'll handle values and indices in v2 will make the issue obsolete. As for fixing it in v1. I think a note in the README is sufficient. I added it here: https://github.com/spatie/enum#override-enum-values

we are for strict types and weak comparison would be against this.

This is true and we'll keep doing it. Strong types all the way.

Regarding the question about making $map a function: if we want to get equality right, we cannot allow returning objects as an enum's value or index. Remember that an enum's value is used to store it in a database. You cannot store its value as an object. The right solution here is to add an abstract method on your enum class, and return anonymous classes implementing that method on a per value basis, as shown here: https://github.com/spatie/enum#enum-specific-methods

I believe that, with the addition to the README, all problems are addressed here, so I'll be closing this issue. Feel free however to keep the conversation going if you've got any more questions.

[brendt]

Let's keep v2 feature requests in the PR discussion: #18

[eclipxe13]

If the conversation leads to a BC and the intention is to allow any type of value.

Could you consider to move static::$map to a protected method? Currently it can only contain scalar values and not objects. I'm thinking about allowing Value Objects in map.

[Gummibeer]

Ok, I read several parts here which belong to different types of issues.

• bug $map can contain string and int values but constructors only support strings
• enhancement allow value objects as value in map

First of all I would like to ask @brendt if the bug is a real bug or if it's intended to allow only string values.

Related to the enhancement: do they have a method in common - like __toString()?
And can you describe an usecase? We work already on a v2 release with a looot of changes. One of the biggest is the instantiation bug fix (string & int) and the drop of $map which is replaced by getValue() and getIndex() methods (current state of development).
Both have a defined return type which we will not drop. If you want to get an instance of any object I would recommend to add a custom getValueObject() method. This could be done once in the Enum or for every value in it's static method like the getNumericIndex() method described in the readme.

My perspective: if the bug is a real bug I would fix it in v1. Objects implementing a __toString() method are possible via the new getValue() method in v2. Every other object is 100% against what an enumerable is and I won't want to support this out of the box.
Like I described in #10 my plan for v2 is to make this library more compatible with the most common native Enum implementations in other languages and the general word and programming definition. So for me it would be a step back to support not stringable objects. Even the separated value is candy on the top.

And last back to the bug and your solution ideas: we are for strict types and weak comparison would be against this. So if we fix this it will be in the way it's done for v2 to add specific checks if something is an index or value.

[Gummibeer]

As a base for this and following discussions about what this/an Enum should support or not I would like to use the enumerable in programming Wikipedia article also referenced by @brendt in #10 .

https://en.m.wikipedia.org/wiki/Enumerated_type

For the upcoming v2 and the related docs at docs.spatie.be I will try to copy this definition into a page to fix it in the package documentation.

[eclipxe13]

It is a current bug, as of the class in README:

$draft = new StatusEnum('1');
$draft->equals(StatusEnum::draft()); // return false, expected true

[eclipxe13]

👍 great.

If static::$map is dropped then would be nice to release version 2 (at least preview) and some information about what is missing to release it.

While this happens:

• could you add third parameter to in_array?
• could you cast to string values obtained from static::map

Thanks again for your work and attention.

[Gummibeer]

@eclipxe13 the v2 branch is mostly finished. It requires review(s) and some real-world testing. It also doesn't have a change/upgrade-log or documentation atm and will not have until release. But the unittests are reworked in total and should show what's possible and what's not. We've also added a new interface which describes all public methods including param types and everything else.

Because it's easy and also done for v2 I will add the in_array(string, array, true) to v1 while casting the values could happen - I will have to check first how easy this would be.

And feel free to use the dev-ft-v2 version to try it out. But beware that this is a new major release with BC and it could be that we introduce new BC during further development.
If you want to I can release a real v2.0.0-dev.1 to allow you to fix the version more precise.