MVVM in Swift iOS

Question

I've been implementing MVVM in Swift. I've looked at several implementations, many of which violate some aspects of MVVM and wanted to have a go with my own version that contains a Web request service.

View:

class BreachView: UIView {
    var nameLabel = UILabel()
    public override init(frame: CGRect) {
        let labelframe = CGRect(x: 0, y: 50, width: frame.width, height: 20)
        nameLabel.frame = labelframe
        nameLabel.backgroundColor = .gray
        super.init(frame: frame)
        self.addSubview(nameLabel)
        backgroundColor = .red
    }

    required init?(coder aDecoder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }
}

ViewController:

class ViewController: UIViewController {

    var breachesViewModel: BreachViewModelType!
    var breachView : BreachView?

    // to be called during testing
    init(viewModel: BreachViewModelType) {
        breachesViewModel = viewModel
        super.init(nibName: nil, bundle: nil)
    }

    // required when called from storyboard
    required init?(coder aDecoder: NSCoder) {
        breachesViewModel = BreachViewModel()
        super.init(coder: aDecoder)
    }

    override func viewDidLoad() {
        super.viewDidLoad()       
        breachesViewModel.fetchData{ [weak self] breaches in
            guard let self = self else {return}
            DispatchQueue.main.async {
                self.updateUI()
            }
        }
    }

    func updateUI() {
        breachView = BreachView(frame: view.frame)
        breachesViewModel.configure(breachView!, number: 3)
        view.addSubview(breachView!)
    }
}

Protocol for dependency injection:

protocol BreachViewModelType {
    func fetchData(completion: @escaping ([BreachModel]) -> Void)
    func configure (_ view: BreachView, number index: Int)
}

ViewModel:

class BreachViewModel : BreachViewModelType {
    var breaches = [BreachModel]()

    init() {
        // add init for ClosureHTTPManager here, to allow it to be teestable in the future
    }

    func fetchData(completion: @escaping ([BreachModel]) -> Void) {
        ClosureHTTPManager.shared.get(urlString: baseUrl + breachesExtensionURL, completionBlock: { [weak self] result in
            guard let self = self else {return}
            switch result {
            case .failure(let error):
                print ("failure", error)
            case .success(let dta) :
                let decoder = JSONDecoder()
                do
                {
                    self.breaches = try decoder.decode([BreachModel].self, from: dta)
                    completion(try decoder.decode([BreachModel].self, from: dta))
                } catch {
                    // deal with error from JSON decoding!
                }
            }            
        })
    }

    func numberItemsToDisplay() -> Int {
        return breaches.count
    }

    func configure (_ view: BreachView, number index: Int) {
        // set the name and data in the view
        view.nameLabel.text = breaches[index].name
    }

}

and HTTP manager

class ClosureHTTPManager {
    static let shared: ClosureHTTPManager = ClosureHTTPManager()

    enum HTTPError: Error {
        case invalidURL
        case invalidResponse(Data?, URLResponse?)
    }

    public func get(urlString: String, completionBlock: @escaping (Result<Data, Error>) -> Void) {
        guard let url = URL(string: urlString) else {
            completionBlock(.failure(HTTPError.invalidURL))
            return
        }

        let task = URLSession.shared.dataTask(with: url) { data, response, error in
            guard error == nil else {
                completionBlock(.failure(error!))
                return
            }

            guard
                let responseData = data,
                let httpResponse = response as? HTTPURLResponse,
                200 ..< 300 ~= httpResponse.statusCode else {
                    completionBlock(.failure(HTTPError.invalidResponse(data, response)))
                    return
            }

            completionBlock(.success(responseData))
        }
        task.resume()
    }
}

Calling the API from

let baseUrl : String = "https://haveibeenpwned.com/api/v2"
let breachesExtensionURL : String = "/breaches"

Any comments on whether the implementation conforms to MVVM or not, typos, changes etc. are appreciated.

Git link: https://github.com/stevencurtis/MVVMWithNetworkService

Answer 1

A couple of thoughts.

• The presence of init(viewModel:) in the view controller (with its associated comment about using this during testing) seems to suggest that you plan on testing the view controller. But one of the central tenets of MVP, MVVM and the like is that the goal is to have a UIKit-independent representation. Part of the reasons we do that is because that is the object that we’ll test, not the view controller.

  Bottom line, I’d be inclined to retire init(viewModel:) from the view controller and restrict the testing to the view model.

  For example, a MVP might look like:

  

  Or, in MVVM, you’ll see structures like:

  

  (Both images from Medium's iOS Architecture Patterns.)

  But in both cases, the mediator (the view model or the presenter), should not be UIKit-specific, and it’s that mediator on which we’ll do our unit testing.

• The view controller has a method updateUI, which is adding the view to the hierarchy. I’d suggest you decouple the initial “configure the view” from the “view model has informed me of some model change”.

• MVVM generally suggests that you’re doing some “data binding”, where the initial configuration of the view controller sets up the connection between presenter events and UIKit control updates and vice versa. It’s hard to tell, but this feels like you’re got the beginning of something with a more MVP je ne sais quoi than MVVM. That’s not necessarily wrong, but they’re just different.

• In BreachViewModel is updating the text property of the UILabel called nameLabel within the view. To my two prior points, the view model, itself, should be UIKit dependent, and it definitely shouldn’t be reaching into a subview of the view and updating the text itself. If this was MVVM, you’d bind the label to the view model and have the update take place that way. If this was MVP, the presenter should just inform the view controller that the value has changed and the view controller would update the UIKit control.

  But avoid having anything UIKit specific in the view model.

• A few observations in fetchData:

  • You have paths of execution where you don’t call the completion handler. You generally always want to call the completion handler, reporting success with data or failure with error information. Perhaps adopt the Result<T, U> pattern you did with ClosureHTTPManager (like we did in the answer to your earlier question).

  • You are decoding your JSON twice. Obviously, you only want to do that once.

• This is personally a matter of taste, but I’m not crazy about the view model doing JSON parsing. That seems like the job of some API layer, not the view model. I like to see the view model limited to taking parsed data, updating models, applying business rules, etc.

I think the aforementioned iOS Architecture Patterns is an interesting discussion of many of these issues. I also think Dave DeLong’s A Better MVC is an interesting read.

---

Generally, if you were going to write MVVM, you’d use a framework like Bond or SwiftRX to facilitate the bindings. But let’s contemplate what a minimalist implementation might look like. (In the absence of these binding networks, this is arguably more MVP than MVVM, but it illustrates the basic idea that this view-model/presenter is where the logic goes and the view controller is just responsible for hooking it up to the view.)

Bottom line, the view controller would basically set up the view model (possibly passing in initial model data), and tell the view model what it wanted to do when the view model wanted to inform us

class BreachViewController: UITableViewController {
    var viewModel = BreachViewModel(model: nil)
    var dimmingView: UIView?

    override func viewDidLoad() {
        super.viewDidLoad()

        // tell view model what you want it to do when the model changes

        viewModel.breachesDidChange = { [weak self] result in
            self?.tableView.reloadData()
        }

        // tell view model that you want it to fetch from the server

        viewModel.fetchBreaches() { [weak self] result in
            if case .failure(let error) = result {
                self?.showError(error)
            }
        }
    }
}

The view model could have a method to perform the request and inform the view (controller) of the changes:

class BreachViewModel {

    var breaches: [Breach]? {
        didSet { breachesDidChange?(breaches) }
    }

    var breachesDidChange: (([Breach]?) -> Void)?

    init(model: [Breach]?) {
        breaches = model
    }

    func fetchBreaches(completion: @escaping (Result<[Breach], Error>) -> Void) {
        ApiManager.shared.fetchBreaches { [weak self] result in
            guard let self = self else { return }

            switch result {
            case .failure:
                self.breaches = nil

            case .success(let breaches):
                self.breaches = breaches.sortedByName()
            }

            completion(result)
        }
    }
}

In this example, the view controller is responsible for the view, the presenter/view model is a small testable class, and network and API logic are encapsulated in separate services.

See https://github.com/robertmryan/Breaches for working demonstration.