Davide Mendolia

FoodTracker refactoring: MVP Part

In this article, we will review FoodTracker Example app provided by Apple as a base to how to refactor and improve maintainability of your app. You can download the original project and follow along this view controller.
We will focus on the MealTableViewController and how to increase his maintainability, we assume that we already have UI tests that cover the scope of the MealTableViewController, that allow us to check that the code as the same functionality before and after the refactoring. You can find the code of the MealTableViewController in the following gist.

MealTableViewController.swift

Before anything else we will set as private all the methods that we can, to improve the readability and reduce coupling with other classes. It’s also a good practice to group methods by protocol they conform to.

To achieve that we can first use the "Generated Interface" tool from XCode to have a bird's-eye view of the class.

internal class MealTableViewController : UITableViewController {

    internal var meals: [FoodTracker.Meal]

    override internal func viewDidLoad()

    internal func loadSampleMeals()

    override internal func didReceiveMemoryWarning()

    override internal func numberOfSectionsInTableView(tableView: UITableView) -> Int

    override internal func tableView(tableView: UITableView, numberOfRowsInSection section: Int) -> Int

    override internal func tableView(tableView: UITableView, cellForRowAtIndexPath indexPath: NSIndexPath) -> UITableViewCell

    override internal func tableView(tableView: UITableView, canEditRowAtIndexPath indexPath: NSIndexPath) -> Bool

    override internal func tableView(tableView: UITableView, commitEditingStyle editingStyle: UITableViewCellEditingStyle, forRowAtIndexPath indexPath: NSIndexPath)

    override internal func prepareForSegue(segue: UIStoryboardSegue, sender: AnyObject?)

    @IBAction internal func unwindToMealList(sender: UIStoryboardSegue)

    internal func saveMeals()

    internal func loadMeals() -> [FoodTracker.Meal]?
}

Everything that is implemented by override or by required by a protocol should have the same visibility of the main type, in this case internal, what remains ?

internal class MealTableViewController : UITableViewController {

    internal var meals: [FoodTracker.Meal]

    internal func loadSampleMeals()

    @IBAction internal func unwindToMealList(sender: UIStoryboardSegue)

    internal func saveMeals()

    internal func loadMeals() -> [FoodTracker.Meal]?
}

Let's set all of them private, and the code compile just fine.
Now, the generated interface only contains the strict minimum.

internal class MealTableViewController : UITableViewController {

    override internal func viewDidLoad()

    override internal func didReceiveMemoryWarning()

    override internal func numberOfSectionsInTableView(tableView: UITableView) -> Int

    override internal func tableView(tableView: UITableView, numberOfRowsInSection section: Int) -> Int

    override internal func tableView(tableView: UITableView, cellForRowAtIndexPath indexPath: NSIndexPath) -> UITableViewCell

    override internal func tableView(tableView: UITableView, canEditRowAtIndexPath indexPath: NSIndexPath) -> Bool

    override internal func tableView(tableView: UITableView, commitEditingStyle editingStyle: UITableViewCellEditingStyle, forRowAtIndexPath indexPath: NSIndexPath)

    override internal func prepareForSegue(segue: UIStoryboardSegue, sender: AnyObject?)
}

Now we can start refactoring, without changing the contract of MealTableViewController class.

First, let's read all the name of functions, that do not have any type related to any library.

  • loadSampleMeals
  • saveMeals
  • loadMeals

They all seems goods candidate to be business rules related to this viewcontroller.

Now we are looking for responsibilities, we like to use the single responsibility principles. What's the name of this class ? MealTableViewController and it extend of UITableViewController.
So we will keep inside this class only elements related to UIKit.

What are the block of code:

  1. Table view data source
  2. Navigation
  3. NSCoding

Table view functions for a tableview controller, perfect.
Navigation is related to UI, we will not talk about them in this article, but be aware that depending on the size of your app it can be interesting to refactor it. So it only remains the NSCoding part, saving and loading meals with NSCoding, they are not UI element, so we will extract them from this class.

A good and common pattern to use is MVP(Model-View-Presenter), we extract these methods to a presenter.

class MealTablePresenter {

    func loadSampleMeals() -> [FoodTracker.Meal]

    func saveMeals(meals: [Meal])

    func loadMeals() -> [FoodTracker.Meal]?
}

You can see that I changed the signature of loadSampleMeals and saveMeals. We will apply here the honest and descriptive name of a function. What's an honest/dishonest function means ? Is that the signature of the function(name, input type, output types) should indicate what the function will do, nothing more nothing less.

  • previously loadSampleMeals had no return value and the name wasn't telling where they were loaded. loadSampleMeals() -> [FoodTracker.Meal] now I know that the meals are simply returned, so it's not loading anymore, but generating sample meals. Let's rename it. So, now we have generateSampleMeals() -> [FoodTracker.Meal] that is like a random function generate an unknown number of meals.

  • saveMeals, had a similar issue you didn't know just by reading the function wich meals were saved.

So we see that meals are present in every function let's improve the naming.

class MealsPresenter {

    func generateSample() -> [FoodTracker.Meal]

    func save(meals: [Meal])

    func load() -> [FoodTracker.Meal]?
}

MealsPresenter is a better name than MealTable, because It abstracts me from how the meal will be display, and I can remove the meals suffix from every method, now that I know the context.

 class MealTableViewController : UITableViewController {

    private var presenter: MealsPresenter = MealsPresenter()
...
}

Here you can see the controller, with a reference to the presenter. Now that we have a presenter, let's extract the presentation logic from the view. So let's find all boolean operation in the viewController, that do not mention UIKit element. There is only one candidate.

    override func viewDidLoad() {
        super.viewDidLoad()
        
        // Use the edit button item provided by the table view controller.
        navigationItem.leftBarButtonItem = editButtonItem()
        
        // Load any saved meals, otherwise load sample data.
        if let savedMeals = presenter.load() {
            meals += savedMeals
        } else {
            // Load the sample data.
            meals += presenter.loadSample()
        }
    }

Because this is linked to the lifecycle of the viewcontroller, we will keep the same naming convention.

// In the view controller
    override func viewDidLoad() {
        super.viewDidLoad()
        
        // Use the edit button item provided by the table view controller.
        navigationItem.leftBarButtonItem = editButtonItem()
        
        presenter.viewDidLoad()
    }

    func showItems(meals: [Meal]) {
        self.meals = meals
    }

Why did I introduce a showItems method, could have called a method on the presenter and set the value directly to meals, yes but this an easy way to introduce async without polluting the signature of the viewcontroller methods.

How to call the showItems method from the presenter when meals are loaded ? I can't reference directly the ViewController, because then the presenter would depend on UIKit, so let's define a protocol, that allow us to call the viewController without knowing is implementation.

protocol MealsUI: class {
    func showItems(meals: [Meal])
}

class MealsPresenter {
    private weak var ui: MealsUI?

    init(ui: MealsUI) {
        self.ui = ui
    }

    func viewDidLoad() {
        // Load any saved meals, otherwise load sample data.
        if let savedMeals = load() {
            ui?.showItems(savedMeals)
        } else {
            // Load the sample data.
            ui?.showItems(generateSample())
        }
    }
...
}

Because we know that we reference cycle between ViewController and Presenter we need to define var ui: MealsUI? as weak in order to don't keep a reference to it when the viewController is not needed anymore.

class MealTableViewController: UITableViewController, MealsUI {
    // MARK: Properties
    private lazy var presenter: MealsPresenter = { return MealsPresenter(ui: self) }()
    private var meals = [Meal]()
...
}

class MealsPresenter {
...

    private func generateSample() -> [Meal] {
    ...
    }

    private func load() -> [Meal]? {
    ...
    }
}

To go further, you could explore dependency injection, extract use cases, and backend or improve the navigation between viewcontrollers.

References: