View controllers and delegation for a weightlifting app in Swift

Question

I've got several UIViewControllers that are trying to do too much so I'm trying to learn how to clean them up so that they only have code that does UI things (ultimately getting things to conform to MVVM).

I know my code needs a lot of improvement so I'm going to start small by focusing on one particular behavior to ask for help with which is saving the user's selection as a user default, but keeping the code that does this out of the UIViewController.

In this example from my app, I have a UITableViewController called FormulasTableViewController which has a list of the formulas the app can use for calculations to choose from:

class FormulasTableViewController: UITableViewController {

  let formulas: [CalculationFormula] = [
    CalculationFormula.epley,
    CalculationFormula.baechle,
    CalculationFormula.brzychi,
    CalculationFormula.lander,
    CalculationFormula.lombardi,
    CalculationFormula.mayhewEtAl,
    CalculationFormula.oConnerEtAl]

  override func viewWillAppear(animated: Bool) {
    super.viewWillAppear(animated)

    refresh()
  }

  override func viewDidLoad() {
    super.viewDidLoad()
  }

  // MARK: - Table view data source

  override func numberOfSectionsInTableView(tableView: UITableView) -> Int {
    return 1
  }

  override func tableView(tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
    return formulas.count
  }

  override func tableView(tableView: UITableView, cellForRowAtIndexPath indexPath: NSIndexPath) -> UITableViewCell {
    let cell = tableView.dequeueReusableCellWithIdentifier("formulasCell", forIndexPath: indexPath)
    let currentFormula = formulas[indexPath.row]
    cell.textLabel?.text = currentFormula.rawValue
    return cell
  }

  func refresh() {
    let preferredFormula = UserDefaultsManager.sharedInstance.preferredFormula
    for index in 0 ..< formulas.count {
      let indexPath = NSIndexPath(forItem: index, inSection: 0)
      let currentFormula = formulas[indexPath.row]
      if let cell = tableView.cellForRowAtIndexPath(indexPath) {
        cell.accessoryType = preferredFormula! == currentFormula ? .Checkmark : .None
      }
    }
  }

  override func tableView(tableView: UITableView, didSelectRowAtIndexPath indexPath: NSIndexPath) {
    tableView.deselectRowAtIndexPath(indexPath, animated: true)

    UserDefaultsManager.sharedInstance.preferredFormula = formulas[indexPath.row]

    refresh()
  }
}

As you can see above in tableView(tableView:didSelectRowAtIndexPath:indexPath), when the user selects one, UserDefaultsManager.sharedInstance.preferredFormula = formulas[indexPath.row] is executed.

class UserDefaultsManager {
  static let sharedInstance = UserDefaultsManager()

  // MARK: Preferred Formula
  private var defaultsFormula: CalculationFormula?

  var preferredFormula: CalculationFormula? {
    get {
      if self.defaultsFormula == nil {
        let defaults = NSUserDefaults.standardUserDefaults()
        if let defaultValue = defaults.objectForKey("preferredFormula") as? String {
          self.defaultsFormula = CalculationFormula(rawValue: defaultValue)
        } else {
          self.defaultsFormula = CalculationFormula(rawValue: "Baechle")
        }
      }
      return self.defaultsFormula
    }

    set {
      self.defaultsFormula = newValue
      let defaults = NSUserDefaults.standardUserDefaults()
      if (self.defaultsFormula == nil) {
        defaults.removeObjectForKey("preferredFormula")
      } else {
        defaults.setObject(self.defaultsFormula!.rawValue, forKey: "preferredFormula")
      }
    }
  }
}

My thinking here is that because selection of a formula is happening in a table which is controlled by UIFormulasTableViewController, UserDefaultsManager.sharedInstance.preferredFormula = formulas[indexPath.row] shouldn't be here.

I think I want to use delegation so that my UserDefaultsManager class will handle the user selection of a table row. But my UserDefaultsManager class is handling a lot of different default settings so if I create a protocol on all the different UIViewControllers that are similar to my UIFormulasTableViewController so UserDefaultsManager can act as their delegate, it's going to conform to a long list of these protocols and that seems silly - but I'm a beginner, so maybe it's not.

To remove this code from the UIViewController would it be reasonable to take this delegation approach?

Answer 1

For me it's a bit misleading that you have CalculationFormula all over the place. I'd create an extra model or struct to handle the formula logic or just hold the data in memory.

To persist the data using UserDefaults is okay, but in your UserDefaultsManager you juggle too much with CalculationFormulas. For example, as a reader I don't understand the intention to name an optional property defaultsFormula when it does not default to anything because it can be nil. So the question is if the default formula should be dynamically set or would it be enough to provide a static default? After this decision, move it over to the model/struct mentioned above. Your preferredForumla property becomes simpler then.

Also move the collection of CalculationFormulas from your view controller to the mentioned file.

With the new class/struct you can make use of the delegation approach.

Answer 2

Here are the changes I ended up making.

I abandoned the idea of using an enum for the formulas. I already had the formulas as NSManagedObjects and now I know how to get what I need with those. I also became acquainted with NSFetchedResultsController which is perfect for this view and what I need to do.

Now, when the user selects one of the formulas, here’s what happens:

  override func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
    tableView.deselectRow(at: indexPath, animated: true)

    let selectedFormula = viewModel.fetchedResultsController.object(at: indexPath) as! Formula
    let newDefaultFormulaUuid = selectedFormula.uuid

    viewModel.onNewFormulaSelected(newDefaultFormulaUuid)

    refresh()
  }

the new formula uuid gets passed to this method in the view model where the new default formula uuid is set as the new default formula uuid:

func onNewFormulaSelected(_ newFormulaUuid: String) {
    UserDefaults.setDefaultFormulaUuid(newFormulaUuid)

    modelLiftEvent.updateLiftEventFormula(newFormulaUuid)
}

The getting and setting of the default formula uuid now is much simpler:

// MARK: Default Formula

static func formula() -> String {
    let storedValue = UserDefaults.standard.string(forKey: UserDefaultsKeys.formulaUuid)
    return storedValue ?? "378E6A58-3A87-4B71-B04A-91106ED8C717" // Epley
}

static func setDefaultFormulaUuid(_ formulaUuid: String) {
    standard.setValue(formulaUuid, forKey: UserDefaultsKeys.formulaUuid)
}

A lot cleaner, I'd say.