On Sun, 11 Sep 2011 20:26:20 +0100, Sebastian Bergmann <sebast...@php.net>
wrote:
[...]
--
[1] https://wiki.php.net/rfc/streamwrapper-factory
[2] http://schlueters.de/~johannes/php/stream_factory.diff
A patch against trunk (or 5.4) would have been nicer. Other than that:
* This patch has a huge BC:
PHP_FUNCTION(stream_wrapper_register)
{
...
- if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ss|l", &protocol,
&protocol_len, &classname, &classname_len, &flags) == FAILURE) {
+
+ uwrap = (struct php_user_stream_wrapper *)ecalloc(1, sizeof(*uwrap));
+
+ if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sf|l", &protocol,
&protocol_len, &uwrap->fci, &uwrap->fcc, &flags) == FAILURE) {
+ efree(uwrap);
It should be changed so that the current form of stream_wrapper_register
is still accepted. I assume this is a mistake because
user_wrapper_get_object still tries to instantiate an object directly if
there's no callback.
* Calling the object handler get_class_name is nicer than using
Z_OBJCE_P(us->object)->name) because it works with more objects.
* You claim this has two advantages -- 1. Allows to inject state into
stream wrappers from other parts of the application without having to
access global state or encode information into the stream uri. 2.
Increases testability of code using stream wrappers.
Could you add some examples substantiating these claims? I have some
doubts.
If your factory is a function, #1 doesn't seem to apply. The only way I
see you can retrieve state to inject is by exactly the same means
available on the wrapper object constructor or stream_open (with the
aggravating factor that in stream_open you have access to the context and
to the URI, but in the factory you have access to none).
If, however, the factory is an object method, I see that you could change
state in the object and the factory method would have access to this new
state (and could inject in the wrapper object).
Claim #2 is more dubious. Adding a level of indirection in object creation
is indeed useful for testing because you can inject to objects that need
to instantiate other objects a different factory that will return mock
objects. However, I don't see how this applies here. The stream wrapper
already doesn't have a fixed dependency; it functions like a basic
factory, instantiating an object of a class you give. You can already swap
the implementation of a stream wrapper by varying the class name upon
registration. So I don't see how you gain anything with this extra
complexity.
--
Gustavo Lopes
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
