Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:112436
MIME-Version: 1.0
References: <d1c4689e-9120-4e90-837c-a1b3eac63cd6@www.fastmail.com> <79044037-4cd4-4065-a3c1-39a4deea3e09@www.fastmail.com>
In-Reply-To: <79044037-4cd4-4065-a3c1-39a4deea3e09@www.fastmail.com>
Date: Sun, 6 Dec 2020 00:23:37 +0100
Message-ID: <CAG9XoMS=7BWpVpbrq2hgtr4owaU5j0pmE9yNhVWX7gpVui2HzA@mail.gmail.com>
To: Larry Garfield <larry@garfieldtech.com>
Cc: php internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000096f4e305b5bfe2e0"
Subject: Re: [PHP-DEV] [RFC] Enumerations
From: benjamin.morel@gmail.com (Benjamin Morel)

--00000000000096f4e305b5bfe2e0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Thanks a lot for this RFC, Larry and Iliya! I can't imagine the amount of
thought and work put into this.
Enums are definitely a most-wanted PHP feature.

I played a bit with the early implementation, and love it so far. Here are
my thoughts on the RFC and the current implementation:

*Serialization*

I guess what it comes down to is whether / how easily a class can return
> an existing instance when asked to unserialize, rather than setting
> properties on an existing instance. That is, given the string
> "C:4:Suit:6:{Spades}" can the class definition return the appropriate
> singleton for Suits::Spades rather than a newly constructed object?
>
> If this proves tricky to implement, it would probably be better to
> forbid serialization than using the default object format and breaking
> the singleton-ness of the case objects.


+1, totally agree with this statement. That's the first thing I tried when
playing with the implementation, and noticed that serialization is not
supported:

    echo unserialize(serialize(Status::Active));
    Notice: unserialize(): Error at offset 0 of 42 bytes

I would definitely expect strict equality to be maintained on enum cases
even after unserialization!

*var_export()*

Currently, var_export() returns a __set_state() syntax:

    var_export(Status::Active);
    Foo\Bar\Status::Active::__set_state(array(
    ))

I guess this should just return Foo\Bar\Status::Active.

*Scalar Enums, ::cases()*

The implementation does not support these yet, so I haven't had a chance to
play with them.
I share Pierre R.'s concerns, though:

Does this mean that an enum can't have two cases with the same primitive
> value ? I would very much being able to do so, for example, when you
> change a name and want to keep the legacy for backward compatibility.


But I'd understand if you just disallow duplicate scalar values altogether,
this is probably the most sensible solution here.
Another idea that comes to mind is that cases() could return an iterator
instead of an array, having the cases as keys and the scalars as values,
but this would probably come as a surprise and be bad for DX.

*Enum & UnitEnum interfaces*

The implementation does not seem to support these yet. Taking the examples
from the RFC:

Suit::Hearts instanceof Enum; // true =3D> Parse error: syntax error,
unexpected token "enum"
Suit::Hearts instanceof UnitEnum; // true =3D> FALSE


Best of luck with the RFC!
=E2=80=94 Benjamin

--00000000000096f4e305b5bfe2e0--