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

Fix TIMOB-23198 - Hyperloop: Android can't access fields with primitive array types #16

Merged
merged 1 commit into from Apr 14, 2016

Conversation

sgtcoolguy
Copy link
Contributor

// Note lack of byte[], since bridge doesn't handle that, we treat it specially in wrap
|| item instanceof int[] || item instanceof double[]
|| item instanceof float[] || item instanceof short[]
|| item instanceof long[] || item instanceof boolean[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked the java docs for primitive types. Do we need to list char here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good catch - but I'm not really sure how we should handle Character and char[].

The issue here is that:

  • the bridge doesn't handle char or char[] conversions
  • char is kind of unique in java in that it's essentially a 16-bit unsigned value meant to represent a Unicode character. So do we try and represent a char[] as String, or do we represent it as something more like short[] or int[]? (short is a 16-bit signed value, int is 32-bit signed) - to me the simple answer is turn char[] into String, but as single char into a String of length 1? Seems a little odd, but also I have seen some unique cases where people treat a char as a number more than a "character".

See http://stackoverflow.com/questions/14204907/what-is-the-difference-between-short-and-character-apart-from-processing

It looks to me like we carefully (or luckily?) avoided char[] and char altogether in terms of surfaced API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As another reference: NativeScript seems to treat char as a String: https://docs.nativescript.org/runtimes/android/marshalling/java-to-js.html

I'm going to guess char as a whole could likely use some loving here so that we:

  • Can accept a JS string for a Character/char/char[] argument in a method call (or assigning to a Character/char/char[] field)
  • Return a JS string when a field is of type Character/char/char[], or a method returns Character/char/char[]

Not sure if we also want to be even more liberal and accept JS Numbers (provided they are in a valid range) for char (or array of numbers for char[])

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The easier to use for the developers the better it is. Pedro once supported full conversation of array, numbers, objects etc. to native types, but Jeff removed it in his implementation. So we need to see how both platforms would do it to achieve as much parity as possible, even in these points.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd propose we merge this for now, and open a JIRA ticket for handling char/char[]/Character separately.

@hansemannn
Copy link
Contributor

Approved!

@hansemannn hansemannn merged commit 248ee38 into tidev:master Apr 14, 2016
@sgtcoolguy
Copy link
Contributor Author

Followup ticket: https://jira.appcelerator.org/browse/TIMOB-23213

@sgtcoolguy sgtcoolguy deleted the TIMOB-23198 branch April 14, 2016 14:57
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

Successfully merging this pull request may close these issues.

None yet

2 participants