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
Let me know if there is anything else I need to do. Nick. On 9 January 2013 06:19, Johannes Schlüter <[email protected]> wrote: > Hi, > > On Tue, 2013-01-08 at 13:47 -0500, Nick Tolomiczenko wrote: > > I recently came across the need to parse JSON in php with very low memory > > consumption. The unfortunately the json_decode() function, currently in > > php, was too memory aggressive when the JSON was huge. I needed a > SAX-like > > incremental parser in php. The C based yajl library, written by lloyd, > was > > exactly what I needed; but I needed it in php. So I wrote the yajl-php > > extension which is available at: https://github.com/neikos/yajl-phpunder > > the PHP license version 3.01 > > That's a nice thing! I had a quick run over the code, a few small > observations: > > * Code layout differs sometimes a bit, would be nice to unify > indention (PHP uses single tabs) > * In at least on error message (php_error_docref) you're ending > with a full stop '.' this would result in an error like "This is > a sentence. in file.php line 3", not beautiful. > * You're using C99 Designated Initializers. PHP uses C89/C90 . > Using C99 limits portability as some compilers, most notably MS > Visual Studio, have limited support. I haven't checked yajl, if > they have the same portability restriction it's fine. See also > > https://wiki.php.net/internals/review_comments#don_t_use_c99_for_portability_reasons > * Including #include <yajl/yajl_parse.h> from php_yajl.h isn't > good. I suggest moving that include and the yajl_parser typedef > to the .c file. See > > https://wiki.php.net/internals/review_comments#php_extnameh_should_be_minimal > * Please also see this note on empty RINIT/RSHUTDOWN: > > https://wiki.php.net/internals/review_comments#empty_rinitrshutdown_functions_should_be_removed > * You are using resources, that is fine while we tend to use > objects instead. > > Looks like a long list, but all small things in fact. > > johannes > > >
