API call to list top stories using dependency injection in iOS with MVVM

Question

I'm currently trying to find out what's the best networking architecture for MVVM applications. I couldn't find many resources and decided to go with dependency injection based architecture as per the very less reading resources that I have found.

I'm not using any 3rd party for web service testing and whatever the networking architecture that I use should be supported to mock the web services as well.

I have found out the DI based networking architecture which was build intended to achieve unit testing according to the Test Pyramid concept at Apple WWDC 2018.

So I have build my networking layer according to that. Following is my APIHandler class.

public enum HTTPMethod: String { case get = "GET" case post = "POST" case put = "PUT" case patch = "PATCH" case delete = "DELETE" } protocol RequestHandler { associatedtype RequestDataType func makeRequest(from data:RequestDataType) -> URLRequest? } protocol ResponseHandler { associatedtype ResponseDataType func parseResponse(data: Data, response: HTTPURLResponse) throws -> ResponseDataType } typealias APIHandler = RequestHandler & ResponseHandler 

Followings are my extensions for request handler and response handler.

extension RequestHandler { func setQueryParams(parameters:[String: Any], url: URL) -> URL { var components = URLComponents(url: url, resolvingAgainstBaseURL: false) components?.queryItems = parameters.map { element in URLQueryItem(name: element.key, value: String(describing: element.value) ) } return components?.url ?? url } func setDefaultHeaders(request: inout URLRequest) { request.setValue(APIHeaders.contentTypeValue, forHTTPHeaderField: APIHeaders.kContentType) } } struct ServiceError: Error,Decodable { let httpStatus: Int let message: String } extension ResponseHandler { func defaultParseResponse<T: Decodable>(data: Data, response: HTTPURLResponse) throws -> T { let jsonDecoder = JSONDecoder() if response.statusCode == 200 { do { let body = try jsonDecoder.decode(T.self, from: data) return body } catch { throw ServiceError(httpStatus: response.statusCode, message: error.localizedDescription) } } else { var message = "Generel.Message.Error".localized() do { let body = try jsonDecoder.decode(APIError.self, from: data) if let err = body.fault?.faultstring { message = err } } catch { throw ServiceError(httpStatus: response.statusCode, message: error.localizedDescription) } throw ServiceError(httpStatus: response.statusCode, message:message) } } } 

Then I loaded my request using APILoader as follows.

struct APILoader<T: APIHandler> { var apiHandler: T var urlSession: URLSession init(apiHandler: T, urlSession: URLSession = .shared) { self.apiHandler = apiHandler self.urlSession = urlSession } func loadAPIRequest(requestData: T.RequestDataType, completionHandler: @escaping (Int, T.ResponseDataType?, ServiceError?) -> ()) { if let urlRequest = apiHandler.makeRequest(from: requestData) { urlSession.dataTask(with: urlRequest) { (data, response, error) in if let httpResponse = response as? HTTPURLResponse { guard error == nil else { completionHandler(httpResponse.statusCode, nil, ServiceError(httpStatus: httpResponse.statusCode, message: error?.localizedDescription ?? "General.Error.Unknown".localized())) return } guard let responseData = data else { completionHandler(httpResponse.statusCode,nil, ServiceError(httpStatus: httpResponse.statusCode, message: error?.localizedDescription ?? "General.Error.Unknown".localized())) return } do { let parsedResponse = try self.apiHandler.parseResponse(data: responseData, response: httpResponse) completionHandler(httpResponse.statusCode, parsedResponse, nil) } catch { completionHandler(httpResponse.statusCode, nil, ServiceError(httpStatus: httpResponse.statusCode, message: CommonUtil.shared.decodeError(err: error))) } } else { guard error == nil else { completionHandler(-1, nil, ServiceError(httpStatus: -1, message: error?.localizedDescription ?? "General.Error.Unknown".localized())) return } completionHandler(-1, nil, ServiceError(httpStatus: -1, message: "General.Error.Unknown".localized())) } }.resume() } } } 

To call my API request. I have created a separate service class and call the web service as follows.

struct TopStoriesAPI: APIHandler { func makeRequest(from param: [String: Any]) -> URLRequest? { let urlString = APIPath().topStories if var url = URL(string: urlString) { if param.count > 0 { url = setQueryParams(parameters: param, url: url) } var urlRequest = URLRequest(url: url) setDefaultHeaders(request: &urlRequest) urlRequest.httpMethod = HTTPMethod.get.rawValue return urlRequest } return nil } func parseResponse(data: Data, response: HTTPURLResponse) throws -> StoriesResponse { return try defaultParseResponse(data: data,response: response) } } 

For syncing both my actual web service methods and mock services, I have created an API Client protocol like follows.

protocol APIClientProtocol { func fetchTopStories(completion: @escaping (StoriesResponse?, ServiceError?) -> ()) } 

Then I have derived APIServices class using my APIClient protocol and implemented my all the APIs there by passing requests and responses. My dependency injection was getting over at this point.

public class APIServices: APIClientProtocol { func fetchTopStories(completion: @escaping (StoriesResponse?, ServiceError?) -> ()) { let request = TopStoriesAPI() let params = [Params.kApiKey.rawValue : CommonUtil.shared.NytApiKey()] let apiLoader = APILoader(apiHandler: request) apiLoader.loadAPIRequest(requestData: params) { (status, model, error) in if let _ = error { completion(nil, error) } else { completion(model, nil) } } } } 

Then I have called this API request on my viewModel class like this.

func fetchTopStories(completion: @escaping (Bool) -> ()) { APIServices().fetchTopStories { response, error in if let _ = error { self.errorMsg = error?.message ?? "Generel.Message.Error".localized() completion(false) } else { if let data = response?.results { if data.count > 0 { self.stories.removeAll() self.stories = data completion(true) } else { self.errorMsg = "Generel.NoData.Error".localized() completion(false) } } else { self.errorMsg = "Generel.NoData.Error".localized() completion(false) } } } } 

Finally call the viewModel's API call from my viewController (View).

func fetchData() { showActivityIndicator() self.viewModel.fetchTopStories { success in self.hideActivityIndicator() DispatchQueue.main.async { if self.pullToRefresh { self.pullToRefresh = false self.refreshControl.endRefreshing() } if success { if self.imgNoData != nil { self.imgNoData?.isHidden = true } self.tableView.reloadData() } else { CommonUtil.shared.showToast(message: self.viewModel.errorMsg, success: false) self.imgNoData = { let viewWidth = self.tableView.frame.size.width let imageWidth = viewWidth - 50 let iv = UIImageView() iv.frame = CGRect(x: 25, y: 100, width: imageWidth, height: imageWidth) iv.image = UIImage(named:"no-data") iv.contentMode = .scaleAspectFit return iv }() self.imgNoData?.isHidden = false self.tableView.addSubview(self.imgNoData!) } } } } 

So I have following questions regarding this approach.

1. I have ended the dependency injection from my APIServices class. Should I bring this all the way up to my viewController class API Call and pass request and params variables from there ?
2. Are there any performance issues in this approach and any improvement to be done?
3. My personal preference is to end all the data related stuffs from the viewModel level and just call the API without passing any parameters from the viewController. Does it wrong? If we pass parameters from the view controller class as per the pure dependency injection way, does it harm to the MVVM architecture?

Answer 1

You asked:

So I have following questions regarding this approach.

1. I have ended the dependency injection from my APIServices class. Should I bring this all the way up to my viewController class API Call and pass request and params variables from there?

No, you don’t want to bring this all the way up to your view controller class. The view model is the owner of model data necessary for the view layer, not the view controller.

I am unsure what you mean by “ended the dependency injection from my APIServices class.”

2. Are there any performance issues in this approach and any improvement to be done?

I do not see any obvious performance issues here. I have quite a few stylistic observations, but nothing major on the basis of what you’ve provided thus far.

3. My personal preference is to end all the data related stuffs from the viewModel level and just call the API without passing any parameters from the viewController. Does it wrong? If we pass parameters from the view controller class as per the pure dependency injection way, does it harm to the MVVM architecture?

Re 3, I would agree that the view model is the source of truth, and that it should not be fetching anything from the view controller (which is, effectively, just part of the “view”). The only time you’re possibly passing data from VC to VM is during the instantiation (it depends upon how you are coordinating transitions from scene to scene), but at that point, the view model is the owner of all model data associated with that view. If you do, for example, a pull to refresh, the VM has everything it needs. Nothing is needed/wanted from VC at that point.

---

In terms of a code review, here are a few observations:

• In defaultParseResponse, I might advise replacing:

  if response.statusCode == 200 … 

  with:

  if 200...299 ~= response.StatusCode … 

  because any 2xx code is success.

• You have the Generel.Message.Error string literal sprinkled throughout the code. Whenever you see a string literal repeated, you really should consider replacing it with a constant. If you ever wanted to change this constant (e.g. fix the typo in “General”), you now have to replace it in multiple places.

• Personally, I would avoid generalized localize() function, as you’re losing the second parameter, the “comment” for the translator. You are relying on the key of the string to tell the translator what the functional intent is, but there’s not enough for the translator to go on. I’d really lean towards something like

  let message = NSLocalizedString("…", comment: "Network error message") 

  Where the “…” is the actual error text in the primary language of the app. That, combined with the “comment” allow the translator a better chance to quickly grok what they have to do, without a deep understanding of the code strings.

• Taking this a step further, I would advise against error message strings, but rather proper error objects. And have your error objects conform to LocalizedError (as described here). That way, you abstract the “what is the user friendly error message” from the “what is the rich error object with all the necessary diagnostic information for the app”.

• The loadAPIRequest can probably be tightened up considerably:

  • There are multiple guard statements that could be consolidated into one.
  • I’d personally make the if response = response as? HTTPURLResponse a guard with early exit.
  • I would avoid sentinel status code of -1. Make it optional instead.
  • I would use a Result type rather than an Error? parameter.
  • I would return the URLSessionTask?, and make it a @discardableResult, in case you want to support cancelation in the future.
  • If JSON decoding fails, you’re throwing away any meaningful information in the error object that would let you diagnose what went wrong.
  • It’s probably beyond the scope of the question here, but if I get a malformed response, I personally would log it in my crash reporting framework so that the development team is proactively notified of the discrepancy between what the client expected and what the server returned.

• I might advise early exit pattern in APIServices.fetchTopStories and ViewModel.fetchTopStories.

• I might advise using a linter, like SwiftLint to ensure consistency in spacing and the like.

• In fetchData, I might decompose the construction of the “no data image” out of the this specific view controller method.