Re: [PEPr] Comment on Web Services::Services_JSON

On 8 Oct 2005 22:36:39 -0000, Justin Patrin <papercrane@reversefold.com> wrote: > > Justin Patrin (http://pear.php.net/user/justinpatrin) has commented on the proposal for Web > Services::Services_JSON. > > Comment: > > 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). > > '/^("|\').+("|\')$/s' > Perhaps you mean: > '/^("|\').+$/s' Strange....my regex seems to have been changed when I submitted it. This should be: '/^("|\').+\1$/s' > > $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); > ++$c; > break; > > And: > $utf8 .= substr($chrs, $c, 2); $c += 1; > Should be: > $utf8 .= substr($chrs, $c, 2); > ++$c; > > preg_match('/^\[.*\]$/s', $str) || preg_match('/^\{.*\}$/s', $str) > can be reduced to: > preg_match('/^([\[\{]).*$/s', $str) And again here: preg_match('/^([\[\{]).*\1$/s', $str) > > "\" should be '\' ....and this was "\\" and '\\' > > 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. > -- Justin Patrin