Skip to content
This repository has been archived by the owner on Jan 18, 2022. It is now read-only.

GTMOAuth2ViewControllerTouch needs refactoring #28

Closed
GoogleCodeExporter opened this issue Aug 11, 2015 · 1 comment
Closed

GTMOAuth2ViewControllerTouch needs refactoring #28

GoogleCodeExporter opened this issue Aug 11, 2015 · 1 comment

Comments

@GoogleCodeExporter
Copy link

This class is way too complicated/overloaded.  As a result, it is hard to 
customize.  

1.  Needs better MVC separation.
2.  There are two classes defined in the same class file 
GTMOAuth2ViewControllerTouch and GTMOAuth2Keychain.
3.  Needs ARC compliance.
4.  The finishedSelector can be removed entirely as developers should use 
blocks instead.  This makes the code simpler and cleaner to work with.
5.  Needs support for Storyboard UIs with segue to this authentication 
controller.

Original issue reported on code.google.com by rob...@visi-call.com on 14 Mar 2014 at 6:22

@GoogleCodeExporter
Copy link
Author

Thanks for your feedback.

> Needs better MVC separation.

Considering there are two separate views and view controllers for the same 
underlying model, and there is some shared logic across platforms, the current 
model has held up reasonably well.

> There are two classes defined in the same class file 
GTMOAuth2ViewControllerTouch and GTMOAuth2Keychain.

Proliferation of source files in shared code makes build issues even more 
challenging. Colocating related classes here is intentional.

> Needs ARC compliance.

The code predates ARC, but it interoperates fine in an ARC application.

> The finishedSelector can be removed entirely as developers should use blocks 
instead.

Choice of blocks or callback selectors depends on the nature of the callback. 
Selectors are still superior where the callback is substantially complex and 
independent of the initiating code.

> Needs support for Storyboard UIs with segue to this authentication controller.

This project does predate Storyboards. It should be updated for those, and for 
modern UI generally.

Original comment by grobb...@google.com on 25 Mar 2014 at 12:04

  • Changed state: WontFix

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant