[OPEN-ILS-DEV] PATCH: osrf_json_utils.h (scientific notation)

[Mike Rylander]

-------UPDATE----------

Bill and I have been discussing this off-line, and we both agree that
such an API addition (but leaving jsonObjectGetNumber() to return a
double) is both warranted and, in at least one case for some new code
we'd like to integrate, needed.  I've taken a look at the code that
would need to change, but it's far into a very busy day for me to
tackle right now.  Bill, if you feel like jumping on this, please just
poke the list.  Scott, likewise.  If no one responds then I'll get out
my pokin' stick and see what I can do.  I think I see the clear path,
it'll just take a bit of refactoring in the parser and such.

--miker

On Dec 12, 2007 11:08 PM, Mike Rylander <mrylander at gmail.com> wrote:
> On Dec 11, 2007 9:18 PM, Scott McKellar <mck9 at swbell.net> wrote:
> > This tiny patch enables the JSON parser to accept a capital 'E' as
> > well as a lower case 'e' in a number expressed in scientific notation.
> >
> > If there is some reason to insist on lower case -- which I doubt --
> > then disregard this patch.
>
> I can think of no reason.  Applied.
>
> >
> > -----------
> >
> > As long as we're on the subject of numeric representations, I'm
> > going to raise an issue that has been bothering me for a while.
> >
> > When we parse a number within a JSON string, we immediately convert it
> > into a double and store it as such within the jsonObject.  Due to the
> > inherent imprecision of floating point, this conversion may change
> > the numeric value.
> >
>
> This has always bugged me too, so I'm all for a plan that fixes this issue.
>
> [snip examples]
>
> > For example, we might want to filter the JSON text so as to remove
> > certain parts of it, leaving the rest intact.  In such a case we
> > would want to preserve the original content unchanged.
> >
>
> This (not using the number in calculations) is nearly always the case
> for Evergreen apps.
>
> > Accordingly, I propose that numeric values be stored as character
> > strings, in order to preserve the original values exactly.  In a
> > context where we don't need to do any arithmetic on it, we can
> > extract it in the original format.  Only when necessary would we
> > convert it to a numeric value.  This way we would avoid losing
> > information when we don't have to.
> >
>
> And only return a converted copy, keeping the original until it is
> later changed with jsonObjectSetNumber().
>
> > The object type would still be JSON_NUMBER, but the internal
> > representation would be different.
> >
> > The strategy would be:
> >
> > 1. Identify all locations that access the value.n member of a
> > jsonObject.
> >
>
> This only happens inside libopensrf, so that's good.
>
> > 2. Rewrite them to call either jsonObjectToSimpleString() or
> > jsonObjectGetNumber(), according to circumstance.
> >
>
> I think jsonObjectGetNumber() will be correct in all cases.
>
> > 3. Modify osrf_json_object.c as needed to store the number as a
> > character string.
> >
>
> So, we let jsonObjectGetNumber() continue to return a double, but
> create that double at call time instead of just retrieving it from the
> (now non-existant) value.n member.
>
> In addition to the double version, we should be able to retrieve a
> native int (as well as send a float with a decimal value of .0 instead
> of forcing to an integer).  So, we need be able to see if we are
> working with a float or an int.  To do that, we test for the existence
> of '.', 'e', or 'E' in the string.  This test can be a function called
> jsonObjectNumericType(), and can do the calculation on demand.
>
> If it's an int, then we can still call jsonObjectGetNumber() and get
> back a double, but there should also be a jsonObjectGetInt(),
> returning an int, so that when we know a priori the type of the number
> value (or we've tested it with jsonObjectNumericType() function) and
> we know we're dealing with an integer, then we don't need to do the
> silly (int)foo cast and we can just use atoi internally.
>
> We'd also want a jsonObjectSetInt() analog to the
> jsonObjectSetNumber() function.  SetNumber would always serialize to a
> float, and SetInt would always serialize to a decimal number.
>
> Thinking a little further, jsonObjectGetInt() could just be a wrapper
> to jsonObjectGetNumber() that applies the (int) cast to the returned
> double.
>
> Thoughts?
>
> > 4. Eliminate the value.n member altogether.  If we overlooked any
> > spots that used it, we'll find them in the next build.
> >
>
> Yes.
>
> > Admitted drawbacks:
> >
> > 1. Storing numbers as character strings would require us to allocate
> > and free buffers, incurring additional overhead.
> >
>
> Luckily, we're already building a buffer while parsing the string, for
> the purpose of converting to a double, so no extra cost there.
>
> > 2. We may not want to take these measures until the legacy JSON
> > code is gone, so that we wouldn't have to change things in both
> > places.
> >
>
> I /think/ we can skip this, because all existing code uses the new
> JSON format, except for the hybrid gateway apache module.  In that
> case it uses the legacy JSON code simply for parsing and serializing
> to legacy clients, so leaving that behavior as-is is really OK.
> What's more is we're moving all client code to the new JSON format in
> on go as far as the ILS code is concerned, and even so, the existing
> behavior does not cause any problems now.
>
>
> > Scott McKellar
> > http://home.swbell.net/mck9/ct/
>
>
>
> --
> Mike Rylander
>  | VP, Research and Design
>  | Equinox Software, Inc. / The Evergreen Experts
>  | phone:  1-877-OPEN-ILS (673-6457)
>  | email:  miker at esilibrary.com
>  | web:  http://www.esilibrary.com
>



-- 
Mike Rylander
 | VP, Research and Design
 | Equinox Software, Inc. / The Evergreen Experts
 | phone:  1-877-OPEN-ILS (673-6457)
 | email:  miker at esilibrary.com
 | web:  http://www.esilibrary.com