Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Proposal to change the way telemetry is initalized #7847

Closed
tobrun opened this issue Jan 25, 2017 · 1 comment
Closed

Proposal to change the way telemetry is initalized #7847

tobrun opened this issue Jan 25, 2017 · 1 comment
Labels
needs discussion telemetry Integration with Mapbox Telemetry libraries

Comments

@tobrun
Copy link
Member

tobrun commented Jan 25, 2017

Some thoughts capturing from a comment on the Telemetry module PR.

Telemetry is currently initialized with:

MapboxTelemetry telem = MapboxTelemetry.getInstance();
telem.initalize(context, accessToken, locationProvider);

I'm not a big fan of having both a getInstance and initalizer method as it defeats the purposes of those methods separately and I feel this is thread unsafe as it can lead to race conditions.

I'm thinking this can be solved in two ways:
We either change the underlying code to add parameters to getInstance and setting those as class members + referencing those solely through INSTANCE class member. An example of this in Mapbox.java

Other idea is to separate the concerns of this class into different components using a design patterns (eg. a factory pattern could serve as a wrapper around the paremeters?). This should remove the requirement of the initialization method.

cc @zugaldia

@tobrun tobrun added needs discussion telemetry Integration with Mapbox Telemetry libraries labels Jan 25, 2017
@tobrun
Copy link
Member Author

tobrun commented Jan 25, 2017

Wrong repo, continuing mapbox/mapbox-java#289

@tobrun tobrun closed this as completed Jan 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs discussion telemetry Integration with Mapbox Telemetry libraries
Projects
None yet
Development

No branches or pull requests

1 participant