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

Use a JSON library, please don't manually create JSON #32

Closed
yaronyg opened this Issue Jul 16, 2015 · 3 comments

Comments

Projects
None yet
3 participants
@yaronyg
Member

yaronyg commented Jul 16, 2015

The code used in Android to talk with the JXCore native layer manually creates JSON objects. This is both error prone and hard to read. It would be better to use a JSON serializer with a native Java object.

@obastemur

This comment has been minimized.

Show comment
Hide comment
@obastemur

obastemur Jul 16, 2015

Member

PS: When performance is the case, instead of creating JSON, you might consider using primitive types for better performance.

Member

obastemur commented Jul 16, 2015

PS: When performance is the case, instead of creating JSON, you might consider using primitive types for better performance.

@yaronyg

This comment has been minimized.

Show comment
Hide comment
@yaronyg

yaronyg Jul 16, 2015

Member

Fair enough but this is JSON for returning data in JXCore native calls. All of our native calls are "chunky" enough that perf isn't an issue. So in that case I would argue that code cleanliness is more important.

Member

yaronyg commented Jul 16, 2015

Fair enough but this is JSON for returning data in JXCore native calls. All of our native calls are "chunky" enough that perf isn't an issue. So in that case I would argue that code cleanliness is more important.

@DrJukka

This comment has been minimized.

Show comment
Hide comment
@DrJukka

DrJukka Aug 11, 2015

Contributor

Did change the implementation to use real JSON APIs, definitely required for string encoding, characters such as line-feed, are not handled right with earlier implementation story_00_juksilve as well as today updated library implementation are both now using real JSON API.

Contributor

DrJukka commented Aug 11, 2015

Did change the implementation to use real JSON APIs, definitely required for string encoding, characters such as line-feed, are not handled right with earlier implementation story_00_juksilve as well as today updated library implementation are both now using real JSON API.

@DrJukka DrJukka closed this Aug 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment