-
-
Notifications
You must be signed in to change notification settings - Fork 131
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
Constant value can be a array #53
Comments
Hi, I'm not sure I understand could you explain a bit more? |
This is an exaggerated example: class Foo extends Enum
{
const VIEW = ['foo', 'bar'];
const EDIT = [1, 2, 3];
} bug: echo Action::VIEW(); // PHP Notice: Array to string conversion |
It will be better use the name of constant: echo Action::VIEW(); // VIEW |
OK I see, we cannot change the existing behavior it would be a BC break. However we could check if the value is an array: in that case there could be, for example, an |
Not sure it's a bug. If people would use it for arrays instead of a single value, then the behaviour could be considered unpredictable, as you now effectively have a set and not an enum; |
@mnapoli If a multidimensional array is used, I advise simply to replace the transformation of the object to a string. public function __toString()
{
return $this->getKey();
} @chippyash you are right, but i think that we should not rule out such an opportunity. |
@chippyash yes that's a good point of view. @peter-gribanov as I said this solution is a BC break, we cannot do that.
Here are our options:
|
3rd option is just to document it. i.e. don't try and use enums as sets, the behaviour is unpredictable. 4th option, reinforce 3rd option and put in a check that ensures that enum values are scalar and not traversable. Throw an exception if contravening. This might be considered BC breakage however. But worth putting on the list for the next version maybe. |
I see nothing wrong with using an array as a value though 🤔 |
Consider this as a traditional explanation of the difference between ENUM and SET as applied to SQL (where they get used a lot,)
Therefore if you want to construct a SET of ENUMs, use an array as a simple collection, or better still some immutable type. (see https://github.com/chippyash/Monad and the Monadic Collection as an example. Other Collections exist. NB. The Monadic collection checks on type, not value, so you'd have to add some additional functionality to ensue set membership wasn't broke. That'd be the same for any set implementation.) |
Thanks for the explanation, personally I'm still not really convinced we must apply the same logic to PHP. |
Why not? In any language (programming or linguistic,) there are some constants. We are discussing a data structure type which is universal across programming languages. To say that we shouldn't do the same in PHP would lead to confusion. The Enum lib does what it says it will do on the tin. We can always pervert PHP code to do something it's not intended to do. Ever seen a routine to morph one class into another? Not supported officially, but PHP let's you do it. Doesn't make it right though. Enum was intended to house scalars, and provide a much needed basic type implementation. That's what most people would expect it to do. Why break with expectations? As it is, the library is clean and simple with a single concern. Perfect imho. |
The only problem we are facing is |
What about nested arrays? Or arrays of objects? Or even just objects? Or resources? How do you come up with a universal __toString() method for all of those? Have a look at https://github.com/chippyash/Monad/blob/master/src/chippyash/Monad/Match.php to see the pain you have to go through to be 'generalist'. But once you start down the route, you have to cover them all. Any way. Good discussion, I'm sure you'll do what you think is right. Thanks for the lib so far. A |
You cannot add objects or resources into a constant though. |
@mnapoli No. You can define constants as a resource http://php.net/manual/en/language.constants.syntax.php
|
I did not try to do it. Most likely this refers to the function |
Anyway - here is an example of how you might do sets by combining a collection and enum |
@peter-gribanov this doesn't apply to class constants. |
@mnapoli Yes. I made a mistake. This only works for global constants. define('FILE_RESOURCE', fopen(__FILE__, 'r'));
var_dump(FILE_RESOURCE);
But you can still use multidimensional arrays as a constant value: class Foo
{
const DATA = [
[
[
[
'foo' => 'bar',
],
],
],
];
} |
@mnapoli you use for debugging, and i use to display a readable value on the screen. class Foo extends Enum
{
const VIEW = 1;
const EDIT = 2;
public function __toString()
{
return 'acme.demo.foo.'.strtolower($this->getKey());
}
} Use translate value in Twig Foo: {{ foo | trans }} |
@peter-gribanov that would still work since you are overriding the method. |
Bug at this line. (proof)
It will be better use the name of constant for
__toString()
.The text was updated successfully, but these errors were encountered: