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

On 10/10/05, Michal Migurski <mike@teczno.com> wrote: > > > 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. > > You must be a C programmer then. ;-) > > Heh. Mainly I like having the strings all in one place. Definitely not a C > programmer. > I've just posted an update with the extraneous sprintf's removed. Thanks. > > > dec() and enc() are just shorthand synonyms for the encode() & decode() > methods. > > I realize this, of course. I was just wondering why there were these > synonyms. If it's to conform to some kind of standard then ok, but > IMHO you really don't need multiple synonyms for a function unless > you're trying to keep backwards compatibility with something. > > No standard, just brevity. Ok. It's not a requirement to change it. > > I'll address some of your other points in this mail as well: > > > 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. ... > > ... I would much > rather have all of your parsing be in the main parsing function than > have those special comment cases. > > The parsing code is fairly complex - by using reduce_string() at line 518, I > immediately cut down on the number of cases I need to check for by stripping > out leading and trailing whitespace & comments. It also quickly removes > easy-to-regexp single-line "//"-style comments prior to char-by-char > parsing. I'm strongly in favor of keeping the call to this method. Ok, I'll leave it alone for now, then. If I want it "fixed" I'll see if I can patch it. ;-) > > > preg_match('/^\[.*\]$/s', $str) || preg_match('/^\{.*\}$/s', $str) > can be reduced to: > preg_match('/^([\[\{]).*\1$/s', $str) > > Not really - that would match "[...[", instead of "[...]". Ah...hehe. Sorry about that. You're right, of course. > > > There seems to be something wrong with your indeting (see end of > decode()). > > It's actually fine - this is due to the switch statement that starts at 375. > > Thank you for your fine-toothed combing, I try. ^_^ Thanks for listening. -- Justin Patrin