-
Notifications
You must be signed in to change notification settings - Fork 48
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
TelemetryLocationEnabler Null Bug #156
Conversation
- initialize telemetryLocationEnabler if it is null at this point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting here that this PR fixes #147
Other than 👇 looks good to me.
@@ -239,6 +239,10 @@ private void unregisterTelemetryReceiver() { | |||
|
|||
private void disableTelemetryLocationState() { | |||
if (isLocationEnablerFromPreferences) { | |||
if (telemetryLocationEnabler == null) { | |||
telemetryLocationEnabler = new TelemetryLocationEnabler(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you extract a Lazy initialization method as suggested here #147 (comment)?
We could extract a Lazy initialization method and use it accordingly in both call sites (when enabling and disabling).
- extract out check logic into separate method - include method within `enableTelemetryLocationState` to prevetn duplicate code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment not blocking the PR.
@@ -253,6 +250,12 @@ private void checkApplicationContext() { | |||
} | |||
} | |||
|
|||
private void checkLocationEnabler() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we're creating a new instance of TelemetryLocationEnabler
if not already created, what about calling this method createLocationEnabler
instead?
- rename method to `createLocationEnabler`
Under certain circumstances,
TelemetryLocationEnabler
is null. Add in check to initialize, before being used.