Skip to content

Location serializer added#591

Merged
cammace merged 1 commit intomasterfrom
cam-location-telem
Oct 6, 2017
Merged

Location serializer added#591
cammace merged 1 commit intomasterfrom
cam-location-telem

Conversation

@cammace
Copy link
Copy Markdown

@cammace cammace commented Oct 6, 2017

No description provided.


HashMap<String, Object> map = new HashMap<>();
// Potentially null values
map.put(ALTITUDE, location.hasAltitude() ? location.getAltitude() : JsonToken.NULL);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why JsonToken.NULL instead of JSONObject.NULL?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Because the values null, not the entire object. It doesn't really matter though, they result in the samething.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wondering why not keeping consistency with the rest of the code then 🤔

public class NavigationLocation {

private static final String UTC_TIME_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSSZ";
private static final String ACCURACY = "horizontalAccuracy";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Almost all of these variables are duplicated.
👀 MapboxEvent and MapboxNavigationEvent

Copy link
Copy Markdown
Author

@cammace cammace Oct 6, 2017

Choose a reason for hiding this comment

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

doesn't matter, only temporary. New library should fix this.

@cammace cammace merged commit 2a4e87a into master Oct 6, 2017
@cammace cammace deleted the cam-location-telem branch October 6, 2017 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants