Re: [RFC] [DISCUSSION] Make constructors and destructors return void

Hey, > Am 18.06.2020 um 17:18 schrieb Benas IML <benas.molis.iml@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 <bobwei9@hotmail.com <ÉÌW°O¾ > 2Ü¿eÖcØmailto:bobwei9@hotmail.com>> wrote: > > Am 17.06.2020 um 01:10 schrieb Benas IML <benas.molis.iml@gmail.com > > <mailto:benas.molis.iml@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 > > <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