nxtbgthng / OAuth2Client

Client library for OAuth2 (currently built against draft 10 of the OAuth2 spec)
855 stars 217 forks source link

Delegate objects are nullified in dealloc() method #163

Open dodikk opened 9 years ago

dodikk commented 9 years ago

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.

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 https://github.com/nxtbgthng/OAuth2Client/pull/155

toto commented 9 years ago

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

dodikk commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 8 years ago

I pushed up #204, making the delegates weak.