rfc:internal_constructor_behaviour
Differences
This shows you the differences between two versions of the page.
Next revision | Previous revision | ||
rfc:internal_constructor_behaviour [2015/03/01 14:39] – created danack | rfc:internal_constructor_behaviour [2017/09/22 13:28] (current) – external edit 127.0.0.1 | ||
---|---|---|---|
Line 3: | Line 3: | ||
* Date: 2015-03-01 | * Date: 2015-03-01 | ||
* Author: Dan Ackroyd, Danack@php.net | * Author: Dan Ackroyd, Danack@php.net | ||
- | * Status: | + | * Status: |
* First Published at: https:// | * First Published at: https:// | ||
Line 12: | Line 12: | ||
i) to make the behaviour of these classes behave more consistently with the behaviour that most people would expect them to have. | i) to make the behaviour of these classes behave more consistently with the behaviour that most people would expect them to have. | ||
- | ii) setting the coding standards | + | ii) setting the behaviour |
If other internal classes that are not listed in this RFC are found to have behaviour that is not consistent with the behaviour listed in this RFC they should be treated as bugs and fixed at the next appropriate release. | If other internal classes that are not listed in this RFC are found to have behaviour that is not consistent with the behaviour listed in this RFC they should be treated as bugs and fixed at the next appropriate release. | ||
Line 21: | Line 21: | ||
For the internal classes: | For the internal classes: | ||
- | * finfo | + | |
- | * PDO | + | * PDO |
- | * Collator | + | * Collator |
- | * IntlDateFormatter | + | * IntlDateFormatter |
- | * MessageFormatter | + | * MessageFormatter |
- | * NumberFormatter | + | * NumberFormatter |
- | * ResourceBundle | + | * ResourceBundle |
- | * IntlRuleBasedBreakIterator | + | * IntlRuleBasedBreakIterator |
when their constructor is called with parameters that cannot be used to create a valid instance of the class, the constructor returns null. This is inconsistent with classes which are declared in userland which are not capable of returning null value, but instead have to indicate an error by throwing an exception. | when their constructor is called with parameters that cannot be used to create a valid instance of the class, the constructor returns null. This is inconsistent with classes which are declared in userland which are not capable of returning null value, but instead have to indicate an error by throwing an exception. | ||
- | Having a different behaviour for internal classes is inconsistent and highly | + | Having a different behaviour for internal classes is inconsistent and highly |
- | ``` | + | < |
$mf = new MessageFormatter(' | $mf = new MessageFormatter(' | ||
if ($mf === null) { | if ($mf === null) { | ||
echo " | echo " | ||
} | } | ||
- | ``` | + | </ |
- | This RFC proposes setting a standard that internal classes should either return a valid instance of the class or throw an exception. Also for the classes listed | + | This RFC proposes setting a standard that internal classes should either return a valid instance of the class or throw an exception. Also for the classes listed to be changed in PHP 7 so that they follow this behaviour. |
=== Factory methods === | === Factory methods === | ||
- | The classes IntlDateFormatter, | + | The classes IntlDateFormatter, |
- | PHP is a multi-paradigm programming language which support | + | PHP is a multi-paradigm programming language which supports |
- | ``` | + | < |
class NumberFormatter { | class NumberFormatter { | ||
static function create($locale , $style , $pattern = null) { | static function create($locale , $style , $pattern = null) { | ||
Line 62: | Line 62: | ||
} | } | ||
} | } | ||
- | ``` | + | </ |
There is also a function `finfo_open` which is also a procedural way of instantiating an finfo object. This function would not be modified. | There is also a function `finfo_open` which is also a procedural way of instantiating an finfo object. This function would not be modified. | ||
Line 71: | Line 71: | ||
The constructors of several classes check the parameters that they are given and just give a warning when they are not acceptable. This leaves the object instantiated but in an unusable state. | The constructors of several classes check the parameters that they are given and just give a warning when they are not acceptable. This leaves the object instantiated but in an unusable state. | ||
- | ``` | + | < |
<?php | <?php | ||
// | // | ||
Line 79: | Line 79: | ||
var_dump($reflection-> | var_dump($reflection-> | ||
//Fatal error: ReflectionFunctionAbstract:: | //Fatal error: ReflectionFunctionAbstract:: | ||
- | ``` | + | </ |
- | This RFC proposes that this behaviour should be changed so that the constructor should throw an exception if the class cannot be created in usuable | + | This RFC proposes that this behaviour should be changed so that the constructor should throw an exception if the class cannot be created in a usable |
The list of classes that show this behaviour is: | The list of classes that show this behaviour is: | ||
- | * UConverter | + | |
- | * SplFixedArray | + | * SplFixedArray |
- | * ReflectionFunction | + | * ReflectionFunction |
- | * ReflectionParameter | + | * ReflectionParameter |
- | * ReflectionMethod | + | * ReflectionMethod |
- | * ReflectionProperty | + | * ReflectionProperty |
- | * ReflectionExtension | + | * ReflectionExtension |
- | * ReflectionZendExtension | + | * ReflectionZendExtension |
- | * Phar | + | * Phar |
- | * PharData | + | * PharData |
- | * PharFileInfo | + | * PharFileInfo |
Line 102: | Line 102: | ||
The class PDORow gives a fatal error when a user attempts to instantiate it. | The class PDORow gives a fatal error when a user attempts to instantiate it. | ||
- | ``` | + | < |
$foo = new PDORow(); | $foo = new PDORow(); | ||
// PHP Fatal error: | // PHP Fatal error: | ||
- | ``` | + | </ |
Fatal errors should only be used for fatal errors. This RFC proposes that the constructor for PDORow should be changed to throw an appropriate exception rather than giving a fatal error. | Fatal errors should only be used for fatal errors. This RFC proposes that the constructor for PDORow should be changed to throw an appropriate exception rather than giving a fatal error. | ||
+ | |||
+ | |||
+ | |||
+ | |||
Line 118: | Line 122: | ||
For the classes that have a factory creation method the code that currently tests against the constructor returning null: | For the classes that have a factory creation method the code that currently tests against the constructor returning null: | ||
- | ``` | + | < |
$mf = new MessageFormatter(' | $mf = new MessageFormatter(' | ||
if ($mf === null) { | if ($mf === null) { | ||
// error handling code | // error handling code | ||
} | } | ||
- | ``` | + | </ |
could be changed to using the factory method: | could be changed to using the factory method: | ||
- | ``` | + | < |
$mf = MessageFormatter:: | $mf = MessageFormatter:: | ||
if ($mf === null) { | if ($mf === null) { | ||
// error handling code | // error handling code | ||
} | } | ||
- | ``` | + | </ |
For the other classes which do not have an equivalent procedural method which will still return null, the user would need to wrap the call to the constructor in a try/catch block. | For the other classes which do not have an equivalent procedural method which will still return null, the user would need to wrap the call to the constructor in a try/catch block. | ||
Line 151: | Line 155: | ||
==== To Existing Extensions ==== | ==== To Existing Extensions ==== | ||
- | The standard of either returning a usuable | + | The standard of either returning a usable |
===== Open Issues ===== | ===== Open Issues ===== | ||
- | If anyone feels strongly about splitting the vote into separate parts, please say so. | ||
+ | Some of the Intl extension code has always had the behaviour of giving an error notice, and also throwing an exception for the same error. This behaviour should be cleaned up for the release of PHP 7, so that either an error is given, or an exception, but never both. | ||
- | ===== Proposed Voting Choices ===== | ||
- | The vote requires 2/3 to pass. | + | ===== Patches and Tests ===== |
- | Should | + | These classes will be corrected by making |
- | These clases will be corrected by making the constructor throw an exception rather than return null if the construction of the object fails. | + | * finfo |
+ | * PDO | ||
+ | * Collator | ||
+ | * IntlDateFormatter | ||
+ | * MessageFormatter | ||
+ | * NumberFormatter | ||
+ | * ResourceBundle | ||
+ | * IntlRuleBasedBreakIterator | ||
- | * finfo | ||
- | * PDO | ||
- | * Collator | ||
- | * IntlDateFormatter | ||
- | * MessageFormatter | ||
- | * NumberFormatter | ||
- | * ResourceBundle | ||
- | * IntlRuleBasedBreakIterator | ||
- | + | These classes would be corrected by detecting the invalid data in the constructor and throwing an exception at object construction time, rather | |
- | These classes would be corrected by detecting the invalid data in the constructor and throwing an exception at object construction time, rather giving an error when the created instance is used. | + | * UConverter |
- | * UConverter | + | * SplFixedArray |
- | * SplFixedArray | + | * ReflectionFunction |
- | * ReflectionFunction | + | * ReflectionParameter |
- | * ReflectionParameter | + | * ReflectionMethod |
- | * ReflectionMethod | + | * ReflectionProperty |
- | * ReflectionProperty | + | * ReflectionExtension |
- | * ReflectionExtension | + | * ReflectionZendExtension |
- | * ReflectionZendExtension | + | * Phar |
- | * Phar | + | * PharData |
- | * PharData | + | * PharFileInfo |
- | * PharFileInfo | + | |
The class PDORow will be changed to give an exception if an attempt is made to instantiate it from userland. | The class PDORow will be changed to give an exception if an attempt is made to instantiate it from userland. | ||
+ | The changes have been made in this branch: https:// | ||
- | ===== Patches and Tests ===== | + | The list of exceptions used are: |
- | I hope to fix all of these in a branch before opening the vote, so that people can see the details of the changes. | + | |
+ | Exception - finfo | ||
+ | |||
+ | IntlException -UConverter, | ||
+ | |||
+ | InvalidArgumentException - SplFixedArray | ||
+ | |||
+ | PDOException - PDO, PDORow | ||
+ | |||
+ | PharException - Phar, PharData, PharFileInfo | ||
+ | |||
+ | ReflectionException - ReflectionExtension, | ||
+ | |||
+ | |||
+ | |||
+ | ===== Voting | ||
+ | |||
+ | |||
+ | Should the standard paradigm for constructors for internal objects be to return a usable instance | ||
+ | |||
+ | <doodle title=" | ||
+ | * Yes | ||
+ | * No | ||
+ | </ | ||
- | At the very least all of the exception to be used for each class will be determined, | + | Voting |
===== Implementation ===== | ===== Implementation ===== | ||
After the project is implemented, | After the project is implemented, | ||
- | - the version(s) it was merged to | + | - the version(s) it was merged to: 7.0 |
- | - a link to the git commit(s) | + | - a link to the git commit(s): http:// |
- | - a link to the PHP manual entry for the feature | + | - a link to the PHP manual entry for the feature: No new manual entry, the changes are conforming to standard practice. |
rfc/internal_constructor_behaviour.1425220798.txt.gz · Last modified: 2017/09/22 13:28 (external edit)