Skip to content
This repository was archived by the owner on Sep 4, 2026. It is now read-only.

nil unsafe delegates and data sources at dealloc - #2

Merged
jwe-apple merged 2 commits into
ResearchKit:masterfrom
wooster:UnsetDelegates
Apr 21, 2015
Merged

jwe-apple merged 2 commits into
ResearchKit:masterfrom
wooster:UnsetDelegates

Conversation

@wooster

@wooster wooster commented Apr 15, 2015

Copy link
Copy Markdown
Contributor

This isn't a comprehensive review. There are probably some I missed.

System objects with unsafe references to their delegates may outlive the
lifetime of their delegates (eg when animating), sending messages to
deallocated instances. To be safe, if you assign your object as a delegate
to an unsafe delegate you also need to manually manage that unsafe
reference and prevent it from outliving your object.

wooster added 2 commits April 14, 2015 18:09
This isn't a comprehensive review. There are probably some I missed.

System objects with unsafe references to their delegates may outlive the
lifetime of their delegates (eg when animating), sending messages to
deallocated instances. To be safe, if you assign your object as a delegate
to an unsafe delegate you also need to manually manage that unsafe
reference and prevent it from outliving your object.

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.

  • (void)startTracking
    {
    if ([CLLocationManager locationServicesEnabled] == YES)
    {
    APCLogDebug(@"Start location tracking");
    •    self.locationManager.delegate = nil;
      
      self.locationManager = [[CLLocationManager alloc] init];
      self.locationManager.delegate = self;

just curious why you're allocating a whole new CLLocationManager every time you start and stop tracking, rather than creating it once and then starting/stopping SignificantLocationChanges as needed...?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That'd be a valid approach as well. Easy to just check if self.locationManager isn't nil if you wanted to do it that way.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Raised #7 for doing this in future.

@jwe-apple
jwe-apple merged commit 2a75142 into ResearchKit:master Apr 21, 2015
@jwe-apple

Copy link
Copy Markdown
Member

Reviewed by Erin, merged.

jwe-apple pushed a commit that referenced this pull request May 22, 2015
Erin-Mounts added a commit to Erin-Mounts/AppCore that referenced this pull request Jun 24, 2015
YuanZhu-apple pushed a commit that referenced this pull request Jul 17, 2015
Refactor valueString methods
Erin-Mounts added a commit to Erin-Mounts/AppCore that referenced this pull request Jul 31, 2015
…en_eligible_to_email_verification

Add Share This App option in verify email
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants