mylogon341 / gtm-oauth2

Automatically exported from code.google.com/p/gtm-oauth2
0 stars 0 forks source link

GTMOAuth2ViewControllerTouch needs refactoring #28

Closed GoogleCodeExporter closed 9 years ago

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