php-enum icon indicating copy to clipboard operation
php-enum copied to clipboard

Constant value can be a array

Open peter-gribanov opened this issue 7 years ago • 22 comments

Bug at this line. (proof) It will be better use the name of constant for __toString().

peter-gribanov avatar May 16 '17 14:05 peter-gribanov

Hi, I'm not sure I understand could you explain a bit more?

mnapoli avatar May 16 '17 14:05 mnapoli

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

peter-gribanov avatar May 16 '17 14:05 peter-gribanov

It will be better use the name of constant:

echo Action::VIEW(); // VIEW

peter-gribanov avatar May 16 '17 14:05 peter-gribanov

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 implode().

mnapoli avatar May 20 '17 15:05 mnapoli

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;

chippyash avatar May 22 '17 04:05 chippyash

@mnapoli If a multidimensional array is used, implode() does not solve the problem. In addition, it will not be very readable.

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.

peter-gribanov avatar May 22 '17 10:05 peter-gribanov

@chippyash yes that's a good point of view.

@peter-gribanov as I said this solution is a BC break, we cannot do that.

__toString() is usually used for debugging so it would make sense to me to dump something useful.

Here are our options:

  1. do nothing and keep the error
  2. dump something useful
  3. ?

mnapoli avatar May 22 '17 19:05 mnapoli

@mnapoli

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.

chippyash avatar May 22 '17 19:05 chippyash

I see nothing wrong with using an array as a value though 🤔

mnapoli avatar May 22 '17 19:05 mnapoli

Consider this as a traditional explanation of the difference between ENUM and SET as applied to SQL (where they get used a lot,)

An ENUM value must be one of those listed in the column definition, or the internal numeric equivalent thereof. The value cannot be the error value (that is, 0 or the empty string). For a column defined as ENUM('a','b','c'), values such as '', 'd', or 'ax' are illegal and are rejected.

A SET value must be the empty string or a value consisting only of the values listed in the column definition separated by commas. For a column defined as SET('a','b','c'), values such as 'd' or 'a,b,c,d' are illegal and are rejected.

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.)

chippyash avatar May 22 '17 20:05 chippyash

Thanks for the explanation, personally I'm still not really convinced we must apply the same logic to PHP.

mnapoli avatar May 22 '17 20:05 mnapoli

@mnapoli

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.

chippyash avatar May 22 '17 20:05 chippyash

The only problem we are facing is __toString() it seems, so it doesn't seem like a good reason to prevent that kind of usage. Let's see if other problems pop up because of arrays.

mnapoli avatar May 22 '17 20:05 mnapoli

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

chippyash avatar May 22 '17 21:05 chippyash

You cannot add objects or resources into a constant though.

mnapoli avatar May 23 '17 06:05 mnapoli

@mnapoli No. You can define constants as a resource

http://php.net/manual/en/language.constants.syntax.php

It is possible to define constants as a resource, but it should be avoided, as it can cause unexpected results.

peter-gribanov avatar May 23 '17 14:05 peter-gribanov

I did not try to do it. Most likely this refers to the function define()

peter-gribanov avatar May 23 '17 14:05 peter-gribanov

Anyway - here is an example of how you might do sets by combining a collection and enum

chippyash avatar May 23 '17 19:05 chippyash

@peter-gribanov this doesn't apply to class constants.

mnapoli avatar May 24 '17 06:05 mnapoli

@mnapoli Yes. I made a mistake. This only works for global constants.

define('FILE_RESOURCE', fopen(__FILE__, 'r'));
var_dump(FILE_RESOURCE);

print

resource(22) of type (stream)

But you can still use multidimensional arrays as a constant value:

class Foo
{
    const DATA = [
        [
            [
                [
                    'foo' => 'bar',
                ],
            ],
        ],
    ];
}

peter-gribanov avatar May 24 '17 07:05 peter-gribanov

__toString() is usually used for debugging so it would make sense to me to dump something useful.

@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 avatar May 24 '17 07:05 peter-gribanov

@peter-gribanov that would still work since you are overriding the method.

mnapoli avatar May 24 '17 08:05 mnapoli