ReadableType does not distinguish between Integers and Doubles
andpor opened this issue · 8 comments
This is Android specific.
While there is specific API to retrieve Int and Double form ReadableArray and ReadableMap, for some odd reason ReadableType enum does not make this separation and provides only Numeric entry. Having Numeric only is insufficient as one cannot make a decision whether to retrieve a Double or Int from the collection based on this enum value. This is needed for such applications as database interfaces where int vs double makes a huge difference.
Can this be fixed quickly?
cc @kmagiera
I do want to point out that ReadableArray is used to communicate with JS, which has only 64-bit floats. So if you need to specify that you want a Java double vs Java int, you will need to send that information separately from the numeric value.
You mentioned this is specific to Android but I imagine iOS would have a similar issue. Does iOS solve this issue in a way we could port to Android?
This cannot be sent separately. The API is SQL...it expects values...it would be ridiculous to come up with any reasonable API and force the API user to provide markers for each numeric value I think. Imagine sending a very long list of numeric values as the input...
problem is I have to bind the values to specific SQL types..so I need to know if something is a Double or and Integer...
there is getDouble and getInt...why generalize the ReadableType to Number ?
Have to check on iOS but seems not to be an issue since values are translated directly and I do not have to query for type...iOS return Objects which can be type checked by implementation after value is retrieved. In Android however, I cannot get the value without knowing the type up front - this is the issue...it would be great the have a generic get(i) or get(key) that returns an Java Object that can then be type checked...
this is a pretty significant limitation on Android...or am I missing something trivial here...?
public interface ReadableArray {
int size();
boolean isNull(int index);
boolean getBoolean(int index);
double getDouble(int index);
int getInt(int index);
String getString(int index);
ReadableArray getArray(int index);
ReadableMap getMap(int index);
ReadableType getType(int index);
Object get(int index) should return Integer or Double or Long or Boolean or String or ReadableArray or ReadableMap or NULL <---this is missing
}
@andpor I think it's a good point.
We used to only have one number type on java side of the bridge and all JS numeric typese were mapped directly to "double". It turned out to be insufficient and we introduced integer as a separate type (with ReadableArray.getInt
and ReadableMap.getInt
). But since we don't use ReadableArray.getType
anywhere in the codebase no one bothered to update it with newly introduced type.
We should update ReadableType
now that we support two different numeric types. Required steps are:
- Update
ReadableType.java
: removeNumber
addDouble
andInteger
- Update implementation of
ReadableArray.getType
andReadableMap.getType
bothe are implemented in NDK (c++). Good place to start looking into it is
Good to hear. The same applies to ReadableMap naturally...while I am at it...is there a reason why Long type is not in the mix with corresponding getLong ? It seems to work fine on iOS. Huge difference is SQLite as columns could be int or long...i have to cast longs to int which is obviously not ideal...you can take a look at my ios/android SQLite plugin:
https://github.com/andpor/react-native-sqlite-storage
@andpor I'm afraid this is not sth we can fix. I thought about it a little more since I posted my answer and IMO if you don't know the context of what type to expect you should just use double.
In JS all numbers are 64bit floating point types (java double equivalent). When those numbers are passed over the bridge we serialize them to JSON. We can then read it expecting to be an integer, but the only way we can learn that the type is an integer is by parsing it. If you'd like to implement db backend and from the JS side you want to store floating point numbers in one column. It may have a terrible effect if you determine the type based on an individual value, as it may happen that this individual value that you wish to be a floating point value will be represented in a form of integer (e.g. you may be using 2.0 literal in JS, but it's going to be send as 2).
getInt
have been introduced for the cases when we know that we expect integer on the native side. It provides type checking so that if you send floating-point number by mistake it would redbox. We use it for sending colors represented as 32bit ints or line number properties. IMO it's not suitable for the usecase you've mentioned and you should either make your API explicitly send an information about the type of the column, and then you getInt/getDouble appropriately or just use getDouble all the time as it would give you the exact value that you send from JS
I agree with @kmagiera since JavaScript does not distinguish between ints and doubles (aside from TypedArrays but that's unrelated...). There is only a Number type so there's no way for us to know if you meant int
or double
.
So if you are working with an API that distinguishes between ints and doubles, you need to be explicit about the intended type of your number. For a SQL API, one way to do this is with a prepared query format string like PostgreSQL's INSERT ($1::int, $2::double) INTO ...
. Then you'd parse the format string and understand whether a numeric value should be treated as an int or double.
But not sure what React Native can do for you here. Fundamentally this seems like an issue working with two different type systems (JS and Java's).
Hi there! This issue is being closed because it has been inactive for a while.
But don't worry, it will live on with ProductPains! Check out its new home: https://productpains.com/post/react-native/readabletype-does-not-distinguish-between-integers-and-doubles
ProductPains helps the community prioritize the most important issues thanks to its voting feature.
It is easy to use - just login with GitHub.
Also, if this issue is a bug, please consider sending a PR with a fix.
We're a small team and rely on the community for bug fixes of issues that don't affect fb apps.