Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow JsonReader to tell if it peeked a long or double #1621

Closed
garydgregory opened this issue Nov 22, 2019 · 10 comments
Closed

Allow JsonReader to tell if it peeked a long or double #1621

garydgregory opened this issue Nov 22, 2019 · 10 comments

Comments

@garydgregory
Copy link

Problem: I need to tell apart long from doubles when generically using a JsonReader.
Example:

{ "amount" : "93.21948702", "count": 12 }

Today I need to know in advance that one is a double and the other an int/long so I can call nextDouble() or nextLong(). I want the reader to provide me enough information so that I can know which one to call or just give me the right object.

Solution 1: Add JSonReader#getPeeked() and make PEEKED_LONG and PEEKED_NUMBER public.

This leave me to do:

        if (reader.getPeeked() == JSonReader.PEEKED_LONG) {
            return Long.valueOf(reader.nextLong());
        }
        return Double.valueOf(reader.nextDouble());

Solution 2: Add JsonReader#getNumber() returns java.lang.Number.
Which leaves me to do:
reader.nextNumber()
and I can deal with a Double or Long down the line.

I can propose a PR but I'd like to know if there are alternatives. Right now I am using, arg, reflection to get to JSonReader.peeked, PEEKED_LONG and PEEKED_NUMBER.

@lyubomyr-shaydariv
Copy link
Contributor

If I'm not mistaken and remember how Gson readers work, you can check whether the peeked token type indicates a number, and then read it as a string. This would allow any parsing strategy since JSON is not restricted only with longs and doubles.

@swankjesse
Copy link
Collaborator

Yep, you can read a numeric type as a string.

@swankjesse
Copy link
Collaborator

Also! If your JSON needs to differentiate between long and double, your JSON is going to cause you grief. In particular you cannot use it with JavaScript where there is no such distinction.

@garydgregory
Copy link
Author

If I'm not mistaken and remember how Gson readers work, you can check whether the peeked token type indicates a number, and then read it as a string. This would allow any parsing strategy since JSON is not restricted only with longs and doubles.

But I do NOT want to read numbers as strings. I am wondering if we are misunderstanding each other. I want to read the data in the best-fit type possible. Right now, I cannot. The API tells me "I peeked a number" (JsonToken.NUMBER) went in fact it could tell me "I peeked a double". Adding JsonToken.DOUBLE would help here.

@garydgregory
Copy link
Author

Also! If your JSON needs to differentiate between long and double, your JSON is going to cause you grief. In particular you cannot use it with JavaScript where there is no such distinction.

Well, I can't very well tell customers that their JSON is "wrong", their REST APIs is what I've got. It specifies doubles and longs in places per their Swagger 2.0 definitions (https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#data-types)

@lyubomyr-shaydariv
Copy link
Contributor

If I'm not mistaken and remember how Gson readers work, you can check whether the peeked token type indicates a number, and then read it as a string. This would allow any parsing strategy since JSON is not restricted only with longs and doubles.

But I do NOT want to read numbers as strings. I am wondering if we are misunderstanding each other. I want to read the data in the best-fit type possible. Right now, I cannot. The API tells me "I peeked a number" (JsonToken.NUMBER) went in fact it could tell me "I peeked a double". Adding JsonToken.DOUBLE would help here.

JSON syntax does not provide any mention on a concrete type for numeric values, therefore JSON numbers are not limited to 64-bit integer or IEEE374 double precision values, so JsonToken.NUMBER is a right choice to represent the peeked numeric token. Detecting the "real" concrete type right in JsonReader would make it bloated (as if there were also JsonToken.INTEGER, JsonToken.FLOAT and more numeric types for other needs). Consider the following code that would never work neither with nextDouble, nor with nextLong nor with nextInt:

final BigInteger valueBefore = new BigInteger(2048, new Random());
final String json = valueBefore.toString(); // produces a valid JSON document
try ( final JsonReader jsonReader = new JsonReader(new StringReader(json)) ) {
	final JsonToken jsonToken = jsonReader.peek();
	assert jsonToken == JsonToken.NUMBER;
	final String stringValue = jsonReader.nextString();
	final BigInteger valueAfter = new BigInteger(stringValue);
	assert valueBefore.equals(valueAfter);
}

Also, reading the numeric token as a string would allow to parse the number using any parsing strategy.

private static Number parseNaively(final String s) {
	@Nullable final Integer i = Ints.tryParse(s); if ( i != null ) { return i; }
	@Nullable final Long l = Longs.tryParse(s); if ( l != null ) { return l; }
	@Nullable final Float f = Floats.tryParse(s); if ( f != null ) { return f; }
	@Nullable final Double d = Doubles.tryParse(s); if ( d != null ) { return d; }
	... BigInteger, BigDecimal and so on
}

In short, reading the number using nextString is much more flexible and extensible (up to "let's consider 1 as a double despite it looks like an integer").

@LorenKeagle
Copy link

This issue is interesting to me from a different perspective.

I have a use case where I only want to perform processing on String values in a JSON document, and pass any primitive values (numbers, booleans, nulls) as-is. I'm looking for the most performant way to do this, without needing to parse any values for these JsonTokens.

Would something like the following guarantee that no internal parsing or 're-serialization' is going to occur for number types?

writer.jsonValue( reader.nextString() );

Since there's no JsonNumber abstraction, and no JsonNumber JsonReader.nextNumber() and no JsonWriter.value(JsonNumber value), is this the best mechanism for generically dealing with numbers from a performance standpoint? I know there is JsonWriter.value(Number value), but getting a java.lang.Number class from a JsonReader would involve an unnecessary parsing operation.

Also, in response to the JSON spec not defining concrete types for numeric values, I think this issue should be interpreted more in regards to the GSON implementation as opposed to the JSON specification. I say this because the JsonReader class actually defines methods for nextDouble(), nextLong(), and nextInt(), but does not provide a nextNumber() method that corresponds to the abstract JSON 'Number' token. With that in mind, I think the question is valid (not just for my own selfish reasons given my use case above!) :-)

@lyubomyr-shaydariv
Copy link
Contributor

@LorenKeagle

Also, in response to the JSON spec not defining concrete types for numeric values, I think this issue should be interpreted more in regards to the GSON implementation as opposed to the JSON specification. I say this because the JsonReader class actually defines methods for nextDouble(), nextLong(), and nextInt(), but does not provide a nextNumber() method that corresponds to the abstract JSON 'Number' token.

What java.lang.Number implementation would you suggest? BigDecimal, a Java built-in that seems to fit the JSON spec the most, also must do parsing. If this implementation is considered too heavy because of many reasons, something like LazilyParsedNumber might be taken into account, but it's really too lazy, and one would have to make a trade-off that wouldn't work for all possible scenarios, or provide dozen of "read overloads" like nextBigInteger(), nextSuperEfficientNumber(), etc that does not really let you getting rid of peeking for the real number token from the token stream (also, what would these overloads do in case of mismatched values -- fail or convert such values silently probably wasting more time for conversion)? I believe this is why JsonParser was not designed to provide such a method to read number implementations delegating the choice to the end user (since String is a value generic representation container). nextDouble(), nextLong() and nextInt() only exist, I believe, because these methods are used widely and fit the most use cases.

@Marcono1234
Copy link
Collaborator

Related to #1267

@eamonnmcmanus
Copy link
Member

Fixed by #1290.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants