Hey Bob,

Your examples (regarding constructors/destructors) do make sense since you
are using these methods for what they should be used: initialization and
freeing resources. But, this is off-topic to the RFC.

This RFC only proposes to enforce no-return rule on
constructors/destructors. And based on my previous reply, top 2,000
Composer packages that have a BC break don't actually return anything
(early returns and returns that don't change code behavior don't count).

Your examples regarding `__toString`, `__invoke` and `__serialize` are also
not relevant to this RFC. Although, I do understand that you were
responding to email and I'm thankful for that :)


Best regards,
Benas

On Thu, Jun 18, 2020, 7:43 PM Bob Weinand <bobw...@hotmail.com> wrote:

> Hey,
>
> Am 18.06.2020 um 17:18 schrieb Benas IML <benas.molis....@gmail.com>:
>
> Hey Bob,
>
> Magic methods are **never** supposed to be called directly (even more if
> that method is a constructor or a destructor). If that's not the case, it's
> just plain bad code. But by enforcing these rules, we make sure that less
> of that (bad code) is written and as a result, we make PHP code less
> bug-prone and easier to debug. That's also most likely the reason why
>
>
> __construct() is invoked directly on parent calls, sometimes to
> reinitialize an object or
> after ReflectionClass::newInstanceWithoutConstructor.
>
> I invoke __destruct() directly when needing an early freeing of existing
> resources.
>
> "ensure magic methods' signature" RFC opted in to validate `__clone`
> method's signature and ensure that it has `void` return type.
>
> Just for the sake of making sure that you understand what I mean, here are
> a couple of examples that show that no magic method is ever supposed to be
> called directly:
> ```php
> // __toString
> (string) $object;
>
>
> I like using ->__toString() in favor of (string) casts when the variable
> is guaranteed to be an object to highlight that and avoid magic-ness.
>
> // __invoke
> $object();
>
>
> Same here, unless the object is a closure.
>
> // __serialize
> serialize($object);
> ```
>
>
> Can't argue much about that one, I never use serialize().
>
> Moreover, by validating constructors/destructors and allowing an explicit
> `void` return type declaration, we are becoming much more consistent
> (something that PHP is striving for) with other magic methods (e. g.
> `__clone`).
>
>
> Yeah, __clone() is odd. No idea why.
>
> Also, saying that "sometimes you have valid information to pass from the
> parent class" is quite an overstatement. After analyzing most of the 95
> Composer packages that had a potential BC break, I found out that either
> they wanted to return early (that is still possible to do using `return;`)
> or they added a `return something;` for no reason. Thus, no libraries
> actually returned something useful and valid from a constructor (as they
> shouldn't).
>
> Last but certainly not least, constructors have one and only one
> responsibility - to initialize an object. Whether you read Wikipedia's or
> PHP manual's definition, a constructor does just that. It initializes. So,
> the PHP manual is perfectly correct and documents the correct return type
> that a constructor should have.
>
>
> It also is generally a bad idea to have side effects in constructors, but
> _sometimes_ it is justified. Only because something mostly is a bad idea,
> it is not always.
> Also note that other languages completely forbid manual ctor calls. But
> PHP doesn't (and for good reason, like after
> using ReflectionClass::newInstanceWithoutConstructor).
>
> Bob
>
> Best regards,
> Benas
>
> On Thu, Jun 18, 2020, 4:06 PM Bob Weinand <bobw...@hotmail.com> wrote:
>
>> > Am 17.06.2020 um 01:10 schrieb Benas IML <benas.molis....@gmail.com>:
>> >
>> > Hey internals,
>> >
>> > This is a completely refined, follow-up RFC to my original RFC. Based
>> on the
>> > feedback I have received, this PR implements full validation and
>> implicitly
>> > enforces `void` rules on constructors/destructors while also allowing to
>> > declare an **optional** explicit `void` return type. Note, that there
>> is a
>> > small but justifiable BC break (as stated by the RFC).
>> >
>> > RFC: https://wiki.php.net/rfc/make_ctor_ret_void
>> >
>> > Best regards,
>> > Benas Seliuginas
>>
>> Hey Benas,
>>
>> I do not see any particular benefit from that RFC.
>>
>> Regarding what the manual states - the manual is wrong there and thus
>> should be fixed in the manual. This is not an argument for changing engine
>> behaviour.
>>
>> Sometimes a constructor (esp. of a parent class) or destructor may be
>> called manually. Sometimes you have valid information to pass from the
>> parent class.
>> With your RFC an arbitrary restriction is introduced necessitating an
>> extra method instead.
>>
>> In general that RFC feels like "uh, __construct and __destruct are mostly
>> void, so let's enforce it … because we can"?
>>
>> On these grounds and it being an additional (albeit mostly small)
>> unnecessary BC break, I'm not in favor of that RFC.
>>
>> Bob
>
>
>

Reply via email to