Comments for "Services_JSON"

» Submit Your Comment
Comments are only accepted during the "Proposal" phase. This proposal is currently in the "Finished" phase.
» Comments
  • Philippe Jausions  [2005-08-11 23:04 UTC]

    In my tests sprintf('"%s"', $str) is much slower than '"' . $str . '"'

    In reduce_string() use an array of patterns instead of 3 independent calls to preg_replace. and simply "return trim($str);" instead of assigning + return.

    Otherwise some CS problems: put spaces before the opening parenthesis after "if" "switch" "for" and the like.

    Doing ++$loop instead of $loop++ in "for" loops is a little bit faster.

    In decode(), use preg_match('/^("|\').+\1$/s') instead of 2 preg_match(). And farther down fix buggy regex to preg_match('/^\(.*\)$/s')

    The ObjectFromJSON class name is not PEAR CS compliant. Any objection to use stdClass instead, although it may be use full to have a specific class for JSON-created objects? You could accept a class name to be used too.

    On the same topic, Services_JSON is illsuited IMO. Maybe PHP_JSON? Anyway, whatever name it will be, the constants and class names will need to be corrected.
  • Michal Migurski  [2005-08-12 18:28 UTC]

    Thanks for the comments. I made the coding standards changes and a few of the preg_* edits. I assume your "buggy regex" referred to the one that matches curly braces, not parentheses, yes? I switched to stdClass as well.

    Not sure about the Services_JSON/PHP_JSON naming, but I have no preference either way.
  • bertrand Gugger  [2005-08-12 21:42 UTC]

    Your class should be named "Services_JSON" or "PHP_JSON" but not "JSON"
    Looks like everything is static, $this only for $this->use ? Is it not quite a constant ?
    I'm still wondering if such a processing for each char (encode strings) could not be reduced ... no dichotomy possible ?
    Anyway, I appreciate your refactoring of this part even if switch(){} is not so popular.
    Hope to use it ... and forget I use it.
    Bon courage.
  • Philippe Jausions  [2005-08-29 20:40 UTC]

    Did you update the link with your changes? I still see an awful lot of sprintf("%s")...

    For the object type in decode(), it could be useful to specify the class to use to generate the instance.

    I am thinking into adding a JSON backend to Services_Webservice (using this package of course...)
  • Arnaud Limbourg  [2005-09-23 09:23 UTC]


    What is needed to move this proposal forward ?
  • Michal Migurski  [2005-10-08 19:00 UTC]

    I will be factoring out a few sprintf's this week, and changing the class name to match the proposal name.
  • Justin Patrin  [2005-10-08 22:36 UTC]

    sprintf('%d', $var) and sprintf('%f', $var) should really be (int)$var and (float)$var. Sprintf is a slow solution and it very rarely, if ever, needed.

    sprintf('{%s}',and '[%s]', same thing. It makes no sense to do simple string inserting with sprintf. Use '{'.(string).'}' and '['.(string).']'

    Why both enc() and encode() and dec() and decode()?

    Again, sprintf("%s:%s"), not ok. This is simple concatenation. Use (string).':'.(string).

    Perhaps you mean:

    $c+=1 should be ++$c

    Please put each line of code on its own line. Ex:
    case '\b': $utf8 .= chr(0x08); $c+=1; break;
    Should be:
    case '\b':
    $utf8 .= chr(0x08);

    $utf8 .= substr($chrs, $c, 2); $c += 1;
    Should be:
    $utf8 .= substr($chrs, $c, 2);

    preg_match('/^\[.*\]$/s', $str) || preg_match('/^\{.*\}$/s', $str)
    can be reduced to:
    preg_match('/^([\[\{]).*\1$/s', $str)

    "\\" should be '\\'

    There seems to be something wrong with your indeting (see end of decode()).

    Is there a reason the string needs to be reduced before being decoded? It seems that if decode() handles comments (and it seems to) then reduce_string() is basically just repeated code.
  • Michal Migurski  [2005-10-10 05:10 UTC]

    Thanks Justin - those all sound like good suggestions. I generally prefer sprintf() because it looks cleaner in my editor, but if there's a performance reason to eschew it I'm happy to switch.

    dec() and enc() are just shorthand synonyms for the encode() & decode() methods.
  • Michal Migurski  [2005-10-10 05:16 UTC]

    reduce_string() is called in two places to handle two possible locations for "/*...*/" style comments - once at the very start of decode(), to account for comments at the start & end of the entire JSON string, and again inside the array/object literal parsing area to account for comments inside brackets.

    This is the only way I know of to remove such comments without accidentally munging strings that may contain "/*" or "*/".
  • bertrand Gugger  [2005-10-13 12:30 UTC]

    Just for uncommenting the sources, why not use :
    preg_replace( '#(?:/\*(?:(?R)|.)*\*/|//.*$)|(([\'"])(?:.*)(?<!\\\\)\2|([^\'"/]+))#msU',
    "$1", $source);