The art of slow and steady refactoring. Move away responsibilities and break down big classes.

The art of slow and steady refactoring. Move away responsibilities and break down big classes.

In my previous article, I described a typical scenario where a manager asks for a new feature. We naively add it in without looking a little bit ahead and thinking about the implication on the code’s health and the architecture in general.

One of the biggest concerns in the iOS realm is the infamous Massive-View-Controller. I will not do another one-post on tackling this issue because you’re not going to solve it in one blog post. There is no magic architecture. No way! It’s a combination of good practice, clean code and principles to follow and keep in mind.

Just a little diversion: we all misunderstood the Apple MVC. That GUI architecture didn’t say you must have one View Controller; you can have tiny MVCs (plural). Multi-MVC is… MVC!

Adding a new field in our view controller and little tiny logic looks like a tweak, nothing to worry about. It’s just a few methods.
It is effortless to make this change and ship in production. Unfortunately, this way can lead to some serious trouble that you can’t really see and predict. We all have done this to close the ticket within the one week sprint, with the blessing of the scrum master and product owner.
It’s outstanding, and we deliver! But this doesn’t stop up to being a good boy scout.

Boy scout rule

Photo by Mael BALLAND on Unsplash

Always leave the campground cleaner than you found it.

Translating to the software engineering: “always check a module in cleaner than when you checked it out”.
Spend some seconds thinking about this simple but powerful principle. It doesn’t ask you to make the whole class or module perfect. But it asks you to make it a little bit better!
This continuous and relentless process gets our codebase better and better every single day.
What does it mean “a little bit better”? Name a variable better, split one long function into two smaller functions. Just make it better. Use your judgment because there are no written rules or a standard to follow.

Massive-View-Controller

What are the problems with massive view controllers? The first issue is confusion. When you have a view controller with 10 or 20 methods, you will ask questions like, what happens if I change this logic? Will I break the view? They are totally legitimate questions because that view controller does a lot of things. And here comes the second problem: responsibilities.
The third problem is testability: when we encapsulate too much, the class tends to hide complexity (and rot), and it’s challenging to change and test it. We end up, then, in edit and pray programming mode.
The remedy for big classes is refactoring. Yeeeah! We need to make a big boy into tiny boys. But, how? There is no unique way, but we can have some guidance.

Single Responsibility Principle (SRP)

A class should have only one reason to change

Oh god, here we go again. SOLID principles! We are sick of hearing these principles and always reading the same examples with squares, rectangles, shapes, etc.
I also agree that the definition is fuzzy and nebulous. What does it mean? Should Every class have only one function?
No, let’s get back to our expenses management app. Let’s have a look at a subset of ExpenseListViewController.

import UIKit

class ExpensesListViewController: UIViewController, UITableViewDelegate, UITableViewDataSource, UITextFieldDelegate {
    @IBOutlet var tableView: UITableView!
    @IBOutlet var filterStackView: UIStackView!
    @IBOutlet var dateField: UITextFieldWithPadding!
    
    var expenses = [Expense]()
    
    lazy var fromDatePicker: UIDatePicker = {
        let picker = UIDatePicker(frame: .zero)
        picker.translatesAutoresizingMaskIntoConstraints = false
        if #available(iOS 13.4, *) {
            picker.preferredDatePickerStyle = .wheels
        }
        picker.datePickerMode = .date
        picker.date = .startOfMonth
        picker.addTarget(self, action: #selector(dateChanged), for: .allEvents)
        return picker
    }()
    
    override func viewDidLoad() {
        super.viewDidLoad()
        configureTable()
        configureFilter()
        fetchExpenses()
    }
    
    func configureTable() {
        tableView.refreshControl = UIRefreshControl()
        tableView.refreshControl?.addTarget(self, action: #selector(handleRefreshControl), for: .valueChanged)
        tableView.estimatedRowHeight = 80
        tableView.rowHeight = UITableView.automaticDimension
    }
    
    func configureFilter() {
        dateField.layer.cornerRadius = 5
        dateField.inputView = fromDatePicker
        dateField.text = formatDate(fromDatePicker.date)
        
        let doneButton = UIBarButtonItem(title: "Done", style: .done, target: self, action: #selector(datePickerDone))
        let toolBar = UIToolbar(frame: CGRect(x: 0, y: 0, width: view.bounds.size.width, height: 44))
        let barTitle = UILabel()
        barTitle.text = "Select Date"
        toolBar.setItems(
            [UIBarButtonItem(customView: barTitle),
             UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target: nil, action: nil),
             doneButton],
            animated: true
        )
        dateField.inputAccessoryView = toolBar
    }
    
    func fetchExpenses() {
        tableView.refreshControl?.beginRefreshing()
        API.shared.getExpenses(from: fromDatePicker.date) { [unowned self] expenses, error in
            if error != nil {
                DispatchQueue.main.async {
                    self.tableView.refreshControl?.endRefreshing()
                    let alert = UIAlertController(title: "Ooops!", message: "Something went wrong. Try again.", preferredStyle: .alert)
                    alert.addAction(UIAlertAction(title: "OK", style: .default, handler: nil))
                    self.present(alert, animated: true, completion: nil)
                    return
                }
            }
            
            self.expenses = expenses
            DispatchQueue.main.async {
                self.tableView.refreshControl?.endRefreshing()
                self.tableView.reloadData()
            }
        }
    }
    
    @objc func handleRefreshControl() {
        fetchExpenses()
    }
    
    @objc func datePickerDone() {
        dateField.resignFirstResponder()
        self.tableView.setContentOffset(
            CGPoint(
                x:0,
                y: tableView.contentOffset.y - (tableView.refreshControl!.frame.size.height)
            ), animated: true)
        fetchExpenses()
    }
    
    @objc func dateChanged() {
        dateField.text = formatDate(fromDatePicker.date)
    }
    
    func formatDate(_ date: Date) -> String {
        let df = DateFormatter()
        df.dateStyle = .medium
        df.timeStyle = .none
        return df.string(from: date)
    }
    
    func formatAmount(_ amount: Int, currency: String) -> String {
        let nf = NumberFormatter()
        nf.minimumFractionDigits = 2
        nf.maximumFractionDigits = 2
        nf.locale = Locale.current
        return "\(currency) \(nf.string(from: NSNumber(value: Double(amount)/100))!)"
    }
    
    func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
        return expenses.count
    }
    
    func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        let cell = tableView.dequeueReusableCell(withIdentifier: "expenseCell", for: indexPath) as! ExpenseCell
        let expense = expenses[indexPath.row]
        cell.merchantLabel.text = expense.merchant
        cell.dateLabel.text = formatDate(expense.date)
        cell.amountLabel.text = formatAmount(expense.amount, currency: expense.currency)
        return cell
    }
    
    func textField(_ textField: UITextField, shouldChangeCharactersIn range: NSRange, replacementString string: String) -> Bool {
        false
    }
}

Can you identify the view controller’s responsibilities?

I can list some of them:

  • Locate API class
  • Threading
  • Format dates
  • Format currency
  • Creating cell

OMG. And this is just a subset because it’s missing the part of creating a new expense from this view controller or deleting one or more of them.

Like a good boy scout, let’s make this code a little bit better.

I don’t think threading should be the responsibility of this view controller. We should ideally move this to a composition root, but we are not here. Yet.

It would be better if, for now, we create a global function to deal with threading. We move this responsibility away from the view controller and build the composition root’s foundation, which doesn’t exist right now.

The ideal spot for this little function for me would be AppDelegate, because it’s the closest point where the app cycle starts and where we should, ideally, compose our app.

func runOnMainThread(_ work: @escaping () -> Void) {
    DispatchQueue.main.async {
        work()
    }
}

Now, where is the DispatchQueue.main.async, I will use this runOnMainThread function. It seems nothing, a waste of time. Trust me, it won’t and you will see in the next article where I will introduce the unit tests and this will become incredibly useful.

Leave a Reply

Your email address will not be published. Required fields are marked *