[OPEN-ILS-DEV] JSON: retire old parser?

[Bill Erickson]

On Fri, Aug 13, 2010 at 10:04 AM, Scott McKellar <mck9 at swbell.net> wrote:

> The file osrf_json_parser.c implements a JSON parser that we don't use any
> more.  Is there any objection to getting rid of it?
>
> The most important function in this file is jsonParseString(), which parses
> a JSON string and produces a jsonObject to represent its structure.  There
> are also two variants: jsonParseStringRaw() and jsonParseStringFmt(), and a
> number of other functions that are only used internally.
>
> This parser is designed internally as an incremental parser, parsing a
> stream of JSON one chunk at a time.  In practice it has always been used to
> parse an entire JSON string at one go; its incremental aspect, so far as I
> know, has never been used.
>
> We now use a different parser, based on recursive descent, implemented in
> osrf_parse_json.c.  The newer parser is faster, and much better at catching
> syntax errors.
>
> We also have a separate parser in jsonpush.c for parsing incrementally.  So
> far we haven't had a need for it, but if (for example) we ever want to parse
> a large JSON file without loading it all into memory, we can do so.
>
> For some months now, nothing outside of osrf_json_parser.c (the old parser)
> has made any calls to the old parser.  It's dead code.
>
> Accordingly I propose to remove osrf_json_parser.c from the Makefiles, and
> remove its function prototypes from the relevant header files.  We don't
> need to remove it from the repository; we can keep it around as a historical
> relic, though we should probably add a prominent comment pointing out its
> defunctness.
>

+1

I don't see any need to keep it around, since the files live on in the
repository history.


>
> Another question is one of timing: should we include these changes in v2.0?
>  I don't greatly care one way or the other, myself.  The old dead code is
> harmless enough, apart from the clutter and the potential for confusion.  I
> could understand a reluctance to make unnecessary changes just before a
> major release.
>


These are the options I see

1. Evergreen 2.0 uses OpenSRF 1.4 (chunking, parser removal, and possibly
net-server gutting go into 1.6)

2. Evergreen 2.0 uses OpenSRF 1.6 (w/ chunking)

3. Evergreen 2.0 uses OpenSRF 1.6 (w/ chunking and removal of dead JSON
parser)

FWIW, chunking has been in trunk for a couple of weeks now and, after the
initial bug squashing, appears to be working fine.

-b

-- 
Bill Erickson
| VP, Software Development & Integration
| Equinox Software, Inc. / Your Library's Guide to Open Source
| phone: 877-OPEN-ILS (673-6457)
| email: erickson at esilibrary.com
| web: http://esilibrary.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20100813/ed18184d/attachment.htm