One of my biggest interest when it comes to coding is code quality and code structure. Code often grows complex over time and it can be hard not introducing bugs and making code that is difficult to refactor, maintain and read. A common issue I think many iOS developers have experienced is related to the UIViewController class. This class is very central when it comes to native iOS development. It handles the logic behind a single screen or parts of a screen displayed to the user. Also Apple does not provide any good guidance to avoid this.
I have based this post around the suggestions from this other blog post where the author discusses common bad practices with over-using the UIViewController:
http://doing-it-wrong.mikeweller.com/2013/06/ios-app-architecture-and-tdd-1.html
He lists a few common uses for the UIViewController which could be considered bad.
- View setup and custom layout code
- Core data code
- Delegate for everything
- Accessing global states (singletons)
- Navigation logic
I mostly agree with the author and want my classes and methods to be as small as possible as well as only have one single responsibility. iOS ViewControllers tend to become way to large and like the author says, if you do not put in effort to restructure and refactor your ViewControllers, your app will so only consist of enormous ViewControllers.
A good starting point for anyone who wants smalled ViewControllers is to install the XCode Alcatraz plugin Luft which warns you (coloring the line number view red) when your ViewControllers get to big. I like the idea!
Here are my thoughts on the five common pitfalls above:
View setup and custom layout code
No matter if you are using storyboards, nib/xib files or setting up your UI via code, with or without auto-layout there tend to be modification, setup or manipulation of UI elements in the ViewController. I firstly recommend that you use auto-layout as this saves you lines of code trying to make the UI adjust in smart ways to have dynamic layouts. This can be difficult and come with trade-offs when developing with Xamarin/C#. But in reducing code complexity and having a dynamic UI, it’s hard to beat.
The author suggest that all custom UI code should always be in a UIView sub class which seems reasonable. At least decouple it from the ViewController in some way. If you handle auto-layout via code, this can also be reduced significantly via help methods or even better extensions to UIView. Here is a reoccurring code I’ve used in several iOS projects, which handles the common task of animating the bottom constraint of a UITableView of match the animating of a keyboard showing:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 |
extension UIViewController { func animateKeyboardBottomConstraintForNotification(notification:NSNotification, constraint:NSLayoutConstraint, animated: Bool, offset:CGFloat) { let animationDuration = animated ? KEYBOARD_ANIMATION_TIME : 0.00001 if let userInfo = notification.userInfo { var keyboardRect = userInfo[UIKeyboardFrameEndUserInfoKey]!.CGRectValue keyboardRect = view.convertRect(keyboardRect, fromView: nil) let newConstant = view.frame.height - keyboardRect.origin.y - offset UIView.animateWithDuration(animationDuration, animations: { () -> Void in constraint.constant = newConstant self.view.layoutIfNeeded() }) } } } |
Core Data code
Even if XCode default places Core Data logic in your AppDelegate or UIViewController, this should of course be placed in a class responsible for managing storage or data. Personally I would place this in a central place in the app. I think Singletons are overused and should be avoided whenever possible. I normally structure my code so that I have instances (not Singleton) in my AppDelgate (which itself is a Singleton). My AppDelegate often look something like this:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 |
@UIApplicationMain public class AppDelegate: UIResponder, UIApplicationDelegate { public var window: UIWindow? // A list of managers (could perhaps also be refactored out) var apiManager: ApiManager! var dataManager: DataManager! var debugManager: DebugManager! var gravatarManager: GravatarManager! let mainStoryboard = UIStoryboard(name: "Main", bundle: nil) public func application(application: UIApplication, didFinishLaunchingWithOptions launchOptions: [NSObject: AnyObject]?) -> Bool { initInstanceVariables() initFirstViewController() saveLaunchOptions(launchOptions, application: application) return true } // MARK: - Public methods public static func sharedDelegate() -> AppDelegate { return UIApplication.sharedApplication().delegate as! AppDelegate } public func logout() { dataManager.clearAll() apiManager.clearTokensAndSave() pushNotificationManager.unregister() cancelAllRequests() } //MARK: - Private help methods private func initInstanceVariables() { debugManager = DebugManager() dataManager = DataManager() apiManager = ApiManager() // ... } private func initFirstViewController() { self.window = UIWindow(frame: UIScreen.mainScreen().bounds) // ... let viewController = storyboard.instantiateViewControllerWithIdentifier(id) self.window?.rootViewController = viewController self.window?.makeKeyAndVisible() } |
By placing the managers as instances in the AppDelegate I have more control of their life cycle. May not differ that much from normal Singletons, but at least now I have central place where I manage and store my managers. I can also easily reach them via AppDelegate.sharedDelegate().dataManager.
Delegate for everything
This is one of the more interesting one I think. A common bad practice is when using UITableViewController or when delegating UITableViewDataSource and UITableViewDelegate to the ViewController:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 |
public override func viewDidLoad() { super.viewDidLoad() tableView.delegate = self tableView.dataSource = self //... } //MARK: - UITableViewDelegate / UITableViewDataSource public func tableView(tableView: UITableView, numberOfRowsInSection section: Int) -> Int { ... } public func numberOfSectionsInTableView(tableView: UITableView) -> Int { ... } //MARK : - Even more Delegates // A long list of methods making your UIViewController file very large and complex |
Here developers often also place other delegates such as UIPickerViewDataSource, UIAlertViewDelegate and custom delegates for handling delegation from other UIViewControllers and views. However Swift makes both structuring delegate implementations much easier. Using Swift it has become a good practice to extend your custom UIViewController for each implementation of a delegate and let that extension implement that delegate. For example:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 |
//MARK: MyTableViewCellDelegate extension MyViewController : MyTableViewCellDelegate { public func didTapAttachment(attachment:Attachment) { viewModel.selectedAttachment = attachment performSegueWithIdentifier(SegueType.AttachmentSegue) } } //MARK: - UIAlertViewDelegate extension MyViewController : UIAlertViewDelegate { public func alertView(alertView: UIAlertView, clickedButtonAtIndex buttonIndex: Int) { if buttonIndex == 0 { doSomething() } else if buttonIndex == 1 { cancelChallengeAfterAlertViewDismissal() } } public func alertViewCancel(alertView: UIAlertView) { cancelChallengeAfterAlertViewDismissal() } } |
This also allows you to place your UITableViewDelegate and UITableViewDataSource extensions in another file. I usually try to place my TableView extensions in one file and the rest of my ViewController extensions in another. The UITableView extension often look something like this:
|
1 2 3 4 5 6 7 8 9 10 11 12 |
extension MyMeetingViewController : UITableViewDelegate, UITableViewDataSource { func setupTableViewAndRefreshControll() { // custom setup, called from viewDidLoad } public func tableView(tableView: UITableView, heightForRowAtIndexPath indexPath: NSIndexPath) -> CGFloat { ... } //... } |
This will often greatly reduce the size of your ViewController class. I can also the move out helper methods that are only used by your extensions. This one of the primary ways to keep your UIViewController clean in my opinion.
Unfortunately the extensions cannot hold any instance variables, so you still have to keep state variables for your UITableView in your base ViewController. You can if you have a lot of instance variables only related and used by your UITableView extension move this to a ViewModel inner class that you at least place in your MyTableViewDataSource class.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 |
extension MyViewController : UITableViewDelegate, UITableViewDataSource { private enum CellType : String { case Name = "nameCell" case Description = "descriptionCell" } private class CellViewModel { var cellType:CellType init(cellType:CellType) { self.cellType = cellType } } private class TableViewModel { var cellViewModelList:[CellViewModel]! // other states for your TableView } } |
This allows you to keep one viewModel class for your ViewController which does not contain any variables related to your tableView. You still however need to keep a reference in your MyViewController as extensions cannot have instance variables.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 |
public class MyViewController: UIViewController { public class ViewModel { var shouldMakeNameCellFirstResponderInViewDidLoad = false var actionList: ActionList? var selectedAction: Action? var isInProgress = false // et.c. } private var tableViewModel = TableViewModel() private var viewModel = ViewModel() } |
Accessing global states (singletons)
The author above discusses that many inexperienced developers tend to access a lot of application wide singletons in order to store or pass data. For example:
|
1 |
(UIApplication.sharedApplication().delegate as! AppDelegate).objectThatINeed |
Even if I would not contribute it to my own list of bad practices to specifically UIViewControllers, it’s still a bad practice. You should not pass data between ViewController this way, and be careful of storing data in singletons for various reasons (see http://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons). Also, as a refactoring maniac I would instantly create a help method for that awful long code line and place it in my AppDelegate.swift file:
|
1 2 3 4 |
static func sharedDelegate() -> AppDelegate { return UIApplication.sharedApplication().delegate as! AppDelegate } |
The use of singletons in your ViewController will introduce more hard coupled links to other part of your application. Instead you want simple small loosely coupled parts that have as few and clear dependencies to other parts as possible. This is a wider discussion I will not fit in this blog post.
Navigation logic
The author also argues that the navigation logic should not be present in a ViewController. Moreover it shall not know what kind of context it is used in (which I agree to). It does not need to know where it was started from, or which view it is embedded in. However, I have not yet though on how or why to refactor out navigation logic. This is perhaps because I am so used to Storyboard where it is difficult to achieve good separation of View and Navigation Logic. I have to look into this more.
Summary
Swift Protocol Extensions have made it easy to separate delegates and parts of code related to UIViewController both into separate parts and files, which makes the code more easy to structure, maintain and read. Using Auto Layout decrease the need for UI logic in your view controllers. Also putting your custom UI setup code either in UIView subclasses and/or creating them in Storyboards or nib/xib files reduce the amount of code in your ViewController. Overall refactoring of duplicate and reusable code into subclasses or extensions can greatly reduce complexity as well. Also making sure that logic for handling Core Data, Networking and other separate task are placed in other locations than in UIViewController should always be the practice.
Improving further
This is my first blog post and there are more interesting topics related to UIViewController, Extensions, Delegates, separation of code that can be discussed. I am always open to discussions and improvement. If you have any comments, questions or even better suggestions on how to keep your UIViewController clean, please post below.