| Both sides previous revisionPrevious revisionNext revision | Previous revision |
| rfc:spl-improvements [2011/12/19 09:13] – [Proposal] levim | rfc:spl-improvements [2017/09/22 13:28] (current) – external edit 127.0.0.1 |
|---|
| ====== Request for Comments: Spl Improvements ====== | ====== Request for Comments: SPL Improvements ====== |
| * Version: 1.0 | * Version: 1.0 |
| * Date: 2011-12-18 | * Date: 2011-12-18 |
| ===== Introduction ===== | ===== Introduction ===== |
| |
| The Standard PHP Library (Spl) is included by default in the latest builds of PHP. It is intended to help solve many common problems. However, it has many problems of its own. This wiki attempts to outline the problems and to propose how they could be fixed. | The Standard PHP Library (SPL) is included by default in the latest builds of PHP. It is intended to help solve many common problems. However, it has many problems of its own. This wiki attempts to outline the problems and to propose how they could be fixed. |
| |
| ===== What is wrong with the Spl? ===== | ===== What is wrong with the SPL? ===== |
| |
| * **The documentation of the Spl is poor in some areas**. This makes it difficult to the Spl adopt into projects. Until recently, the Spl data-structures did not document which exceptions they throw and when. That effort is still incomplete as of this writing. A lack of examples is another documentation problem. | * **The documentation of the SPL is poor in some areas**. This makes it difficult to adopt the SPL into projects. Until recently, the SPL data-structures did not document which exceptions they throw and when. That effort is still incomplete as of this writing. A lack of examples is another documentation problem. |
| * **The exceptions are somewhat ambiguous.** Part of this problem is due to the aforementioned documentation issue. However, the names of some of the exceptions are misleading. | * **The exceptions are somewhat ambiguous.** Part of this problem is due to the aforementioned documentation issue. However, the names of some of the exceptions are misleading. |
| * [[http://php.net/manual/en/class.outofrangeexception.php|OutOfRangeException]]. The name of OutOfRangeException implies that it should be thrown when you give a value that is greater than the maximum or lower than the minimum. After long consideration, collaboration and examination of its name, documentation and how it is used in the Spl, it seems that it should be used to indicate that you have given an array index of the incorrect type. The general consensus among those who discussed the topic was that this exception is poorly named because it indicates that it should be used when you provide an index that is out of the bounds of the array. That is what [[http://php.net/manual/en/class.outofboundsexception.php|OutOfBoundsException]] is for. | * [[http://php.net/manual/en/class.outofrangeexception.php|OutOfRangeException]]. The name of OutOfRangeException implies that it should be thrown when you give a value that is greater than the maximum or lower than the minimum. After long consideration, collaboration and examination of its name, documentation and how it is used in the SPL, it seems that it should be used to indicate that you have given an array index of the incorrect type. The general consensus among those who discussed the topic was that this exception is poorly named because it indicates that it should be used when you provide an index that is out of the bounds of the array. That is what [[http://php.net/manual/en/class.outofboundsexception.php|OutOfBoundsException]] is for. |
| * [[http://php.net/manual/en/class.domainexception.php|DomainException]]. An exception in the domain is very general. Couple this with the existence OutOfRangeException and InvalidArgumentException and what should DomainException be used for? It is unclear. DomainException is used only in [[http://php.net/manual/en/splfileobject.setmaxlinelen.php|SplFileObject::setMaxLineLen]] (undocumented, but committed). Based on its usage, it could be an InvalidArgumentException instead. | |
| * **The data-structures are inconsistent in how they act when in the same situation.** Consider the following example where an [[http://php.net/manual/en/class.spldoublylinkedlist.php|SplDoublyLinkedList]] and an [[http://php.net/manual/en/class.splfixedarray.php|SplFixedArray]] encounter the same problem: an index greater than the size of the container was accessed. | |
| * <code php><?php | |
| try { | |
| $linkedList = new SplDoublyLinkedList(); | |
| $linkedList[1]; | |
| } catch(Exception $error) { | |
| echo get_class($error) . ': ' . $error->getMessage(). "\n"; | |
| } | |
| try { | |
| $fixedArray = new SplFixedArray(); | |
| $fixedArray[1]; | |
| } catch(Exception $error) { | |
| echo get_class($error) . ': ' . $error->getMessage(). "\n"; | |
| } | |
| ?></code>The result: <code text>OutOfRangeException: Offset invalid or out of range | |
| RuntimeException: Index invalid or out of range</code> They do not throw the same exception. Furthermore, SplDoublyLinkedList throws an exception that inherits from LogicException when it is not a logical exception but a runtime one. | |
| * **The data-structures do not throw the most appropriate and specific exceptions they can.** Throwing specific exceptions makes it easier to debug applications. Consider the following example using an SplDoublyLinkedList where an attempt is made to access an out-of-bounds index, to set an index using a class, and to call pop on an empty container: <code php><?php | |
| $linkedList = new SplDoublyLinkedList(); | |
| |
| try { | * [[http://php.net/manual/en/class.domainexception.php|DomainException]]. An exception in the domain is very general. Couple this with the existence of both OutOfRangeException and InvalidArgumentException and what should DomainException be used for? It is unclear. DomainException is used only in [[http://php.net/manual/en/splfileobject.setmaxlinelen.php|SplFileObject::setMaxLineLen]] (undocumented, but committed). Based on its usage, it could be an InvalidArgumentException instead. |
| $linkedList[1]; | |
| } catch(Exception $error) { | |
| echo get_class($error) . ': ' . $error->getMessage() . "\n"; | |
| } | |
| |
| try { | * **It lacks a high-level API for a low-level implementation of an array.** The major goal of the SPL is to solve common problems. PHP arrays perform multiple duties which works well for most cases. However, a more narrow structure is sometimes needed. SplFixedArray does not quite solve all the needs in this area. If an application works with a large amount of sequential data but with an unknown size, it is difficult to use SplFixedArray. Creating a structure that grows according to needs could be very helpful. Internally it could use SplFixedArray to leverage code reuse, as long as the external API is appropriate. |
| $linkedList[new StdClass()] = 'class'; | |
| } catch(Exception $error) { | |
| echo get_class($error) . ': ' . $error->getMessage() . "\n"; | |
| } | |
| | |
| try { | |
| $linkedList->pop(); | |
| } catch(Exception $error) { | |
| echo get_class($error) . ': ' . $error->getMessage() . "\n"; | |
| } | |
| ?></code> The result: <code text>OutOfRangeException: Offset invalid or out of range | |
| OutOfRangeException: Offset invalid or out of range | |
| RuntimeException: Can't pop from an empty datastructure</code> | |
| - Accessing an out-of-bounds index led to an incorrect exception type of OutOfRangeException, a child of LogicException. The actual problem encountered was not a logical exception, but a runtime one. | |
| - Trying to set an index using an invalid type (a class) led to an OutOfRangeException. The name is ambiguous, but the meaning is correct. The message that the exception provides is the real problem here. It does not provide what went wrong but presents two options. | |
| - Popping from an empty container caused a generic RuntimeException to be thrown. The more correct exception would be [[http://php.net/UnderflowException|UnderflowException]]. | |
| * **The data-structures violate good programming practices.** | |
| * [[http://php.net/manual/en/class.splstack.php|SplStack]] and [[http://php.net/manual/en/class.splqueue.php|SplQueue]]. SplStack and SplQueue are fully exposed [[http://php.net/manual/en/class.spldoublylinkedlist.php|SplDoublyLinkedList]]s. This means you can use an SplStack or SplQueue just as you would use an array. This exposes too much of the implementation to the user and could be a source of bugs. | |
| * [[http://php.net/manual/en/class.splobjectstorage.php|SplObjectStorage]]. SplObjectStorage violates the idea of single responsibility. It is performing duties as a Map and a Set. The API is difficult to use because of this dual-identity. | |
| ===== Proposal ===== | ===== Proposal ===== |
| |
| * **Modifying the data-structures to throw more specific exceptions.** | * **Modifying the data-structures to throw more specific exceptions.** |
| * SplDoublyLinkedList: | * SplDoublyLinkedList: |
| * Should throw an UnderflowException when you call bottom, peek, pop, shift, and top on an empty container. | * Should throw an UnderflowException instead of RuntimeException when you call bottom, peek, pop, shift, and top on an empty container. |
| * SplFixedArray: | * SplFixedArray: |
| * Should throw OutOfBoundsException when an index that is a valid type but is out-of-bounds is used in any operation. | * Should throw OutOfBoundsException instead of RuntimeException when an index that is a valid type but is out-of-bounds is used in any operation. |
| * Should throw OverflowException when you attempt to add a new element to the container. | * Should throw OverflowException instead of RuntimeException when you attempt to add a new element to the container. |
| * **Modifying the exceptions message to be more clear.** | * **Modifying the exceptions message to be more clear.** |
| * Accessing an invalid index type should give a message similar to "Invalid index type: expected [valid types]." | * Accessing an invalid index type should give a message similar to "Invalid index type: expected [valid types]." |
| * Accessing an out-of-bounds index should give a message similar to "Index out-of-bounds." | * Accessing an out-of-bounds index should give a message similar to "Index '$index' is out-of-bounds." |
| * **[[http://php.net/manual/en/class.splobjectstorage.php|SplObjectStorage]] should be split into two structures, a Map and a Set.** SplObjectStorage should then be deprecated and eventually removed. | |
| |
| The problems that cannot be fixed without breaking backwards compatibility should be carefully discussed and examined. A plan to correct them should then be created along with a roadmap for when they will be implemented. | The problems that cannot be fixed without breaking backwards compatibility should be carefully discussed and examined. A plan to correct them should then be created along with a roadmap for when they will be implemented. |
| |
| Some problems that need to be talked about and resolved: | Some problems that need to be talked about and resolved: |
| * **Exceptions.** Each extension in the Spl should well defined and documented. This includes using an example of when the exception could be used. All of the Spl classes should be refactored to align their behavior with the result of this discussion. Note: The examples of exceptions being used could be showing an Spl class throwing the given exception. It would help promote understanding of the Spl structures while clarifying the use of the exception. | * **Exceptions** |
| * **APIs of SplStack and SplQueue.** This author believes that their APIs should be trimmed back. They should not publicly inherit from SplDoublyLinkedList. If that causes an issue, then a discussion should be held on other ways to prevent the misuse of SplStack and SplQueue. | * Each exception in the SPL should be well defined and documented. This includes using an example of when the exception could be used. Note: The examples of exceptions being used could be showing an SPL class throwing the given exception. It would help promote understanding of the SPL structures while clarifying the use of the exception. |
| * **Should the Spl be namespaced?** | * All of the SPL classes should be refactored to align their behavior with the exception clarifications. |
| * If large-scale backwards compatibility is broken, it could be detrimental to do so all at once. However, doing it in pieces is not really reasonable. Namespacing the Spl could allow all of the changes to happen without breaking backwards compatibility because the current Spl is not namespaced. | * **Should the SPL be namespaced?** |
| * This could be the beginning of migrating existing PHP extensions to namespaces. If this works well, then there is a viable way to safely break backwards compatibility and could be used in many other areas where breaking backwards compatibility is undesirable. | * If large-scale backwards compatibility is broken, it could be detrimental to do so all at once. However, doing it in pieces is not really reasonable. Namespacing the SPL while keeping the old SPL in place could allow all of the changes to happen without breaking backwards compatibility because the current SPL is not namespaced. |
| | * This could be the beginning of migrating existing PHP extensions and functions to namespaces. If this works well, then there is a viable way to safely break backwards compatibility and could be used in many other areas where breaking backwards compatibility is undesirable. |