On Wed, 2013-01-09 at 17:15 -0500, Nick Tolomiczenko wrote:
> Hi Johannes,
>
> I made all changes except the last (optional) one regarding the use of
> objects instead of resources. I will do that in a subsequent release if
> that's okay. I pushed the changes to https://github.com/neikos/yajl-php.git
I wouldn't change after a release - either stick with the resource-based
approach or go to an OO interface. I for one could imagine having an
abstract base class the user can extend to create his parser ... on the
other hand the current approach allows things like
yajl_set_string_handler($p, function() { echo "yay, a string"; });
which also is nice ... so that's a design decision you have to take :-)
Ah and another comment: You can simplify the code a tiny bit by using
the "f" modifier for zend_parse_parameters, this gives you the function
information and function pointer. There is one caveat, though: When
storing it you have to mind the refcount of a potential object. Stealing
my code from https://github.com/johannes/php-test-helpers this looks
something like this:
typedef struct {
zend_fcall_info fci;
zend_fcall_info_cache fcc;
} handler_t;
PHP_FUNCTION(foo) {
zend_fcall_info fci;
zend_fcall_info_cache fcc;
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "f", &fci, &fcc)
== FAILURE) {
return;
}
handler->fci = fci;
handler->fcc = fcc;
Z_ADDREF_P(handler->fci.function_name);
#if PHP_VERSION_ID >= 50300
if (handler->fci.object_ptr) {
Z_ADDREF_P(handler->fci.object_ptr);
}
#endif
}
void some_func() {
zend_fcall_info_argn(&handler->fci TSRMLS_CC, 1, &arg);
zend_fcall_info_call(&handler->fci, &handler->fcc, &retval, NULL
TSRMLS_CC);
zend_fcall_info_args_clear(&handler->fci, 1);
}
void free_handler() {
if (handler->fci.function_name) {
zval_ptr_dtor(&handler->fci.function_name);
handler->fci.function_name = NULL;
}
#if PHP_VERSION_ID >= 50300
if (handler->fci.object_ptr) {
zval_ptr_dtor(&handler->fci.object_ptr);
handler->fci.object_ptr = NULL;
}
#endif
}
Benefit is that the user gets a more consistent error message
(zend_parse_parameters will emit one, being the same for all extensions
using this) and by using the stored fcc one safes some lookup time
afterwards making it a tiny bit faster. Probably you won't need 5.2
compatibility, then the #ifdef's can go. (also see
php-src/README.PARAMETER_PARSING)
johannes
--
PECL development discussion Mailing List (http://pecl.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
