Skip to content

Delegate objects are nullified in dealloc() method - #163

Open
dodikk wants to merge 14 commits into
nxtbgthng:developfrom
healthjoy-ios-opensource:HJ/FixDealloc
Open

Delegate objects are nullified in dealloc() method#163
dodikk wants to merge 14 commits into
nxtbgthng:developfrom
healthjoy-ios-opensource:HJ/FixDealloc

Conversation

@dodikk

@dodikk dodikk commented Apr 1, 2015

Copy link
Copy Markdown

Since delegates are __unsafe_unretained they should be set to nil manually whenever a corresponding object is released.
I've added dealloc method to all classes where .delegate = self; statement is used.

  • NXOAuth2Account
  • NXOAuth2Request

The users have to do that as well, so this should be mentioned in the README, I guess.

Note : this pull request is based on from #155

Comment thread NXOAuth2Account+Private.h

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.

To keep in line with the framework this should be called NXOAuth2Application

@toto

toto commented Apr 27, 2015

Copy link
Copy Markdown
Contributor

Generally we could convert the delegates to weak at some point. iOS 4 compatibility is really not that important anymore

@dodikk

dodikk commented Apr 28, 2015

Copy link
Copy Markdown
Author

Generally we could convert the delegates to weak at some point.

That would be a lot better. Still, I did not feel confident enough to introduce such major changes since I have not fully understood the instance ownership model of the library. Neither I do now. Meaning, there are

So I've made the changes as conservative and safe as I could.

@dodikk

dodikk commented Apr 28, 2015

Copy link
Copy Markdown
Author

P.S. Most of UIKit classes still use unsafe_unretained delegate properties for backward compatibility. UITableView, for example.

Moreover, it does not matter that much for this library since NSNotificationCenter is used for interactions with the client's code.

@toto

toto commented Apr 28, 2015

Copy link
Copy Markdown
Contributor

True. You can use NXOAuth2Client on it's own without the NXOAuth2AccountStore and friends (indeed that was how it was first built) and in that case it works with delegates. If you use the higher level API as is appropriate in most use cases it's indeed not very relevant.

@sirnacnud

Copy link
Copy Markdown
Contributor

I pushed up #204, making the delegates weak.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants