I recently came across an interesting post on the r/swift subreddit that featured an example of a “Clean Code” project, which doesn’t happen all too often. I was intrigued and decided to check it out on GitHub, then proceeded to download it, set it up, and try it myself. At first glance, the code was cumbersome to read, and I was somewhat confused, but after downloading it and spending some time in it, all the pieces fell together, and the project seems to accomplish what it was trying to accomplish.
The part of the app that intrigued me most was how complex the application’s networking portion was: how can two simple network fetches span so many files and be so hard to understand? I decided I wanted to clean up the Network layer, modularize it, and make a few other small improvements to composition and UI. I created a separate sample app to showcase these changes. Links to my sample project (which includes all the code I will mention below) and the original project are at the bottom of the article.
Network Layer — Removing Nesting and Types
The original project’s network layer provides a high degree of composability through protocols and types that each take care of one small function.
It goes (roughly) like this:
NetworkManager -> RequestManager -> RequestProtocol -> DataParser -> DataSource -> Repository -> UseCase
Still with me? Each one of these types is responsible for a part of the network process, e.g., the DataParser is responsible for parsing the data. This means that if you wanted to change how the data is parsed, you could do that by passing in a new DataParser — this makes the code composable, which is a good thing!
However, this is rather difficult to read because they are nested inside of each other in a way that makes seeing the big picture hard to understand. Each of these lives in its own file, and many of them are injected through the Swinject Resolver, which makes piecing together exactly how this works surprisingly difficult. As one commenter in r/swift put it, it adds a level of “indirection” to the code.
Even after adding all of these protocols and types to increase flexibility in the code, most of these simply have default values that aren’t even passed in through constructors. The DataParser is just hardcoded in there, and the oddest example is the RequestProtocol.request(), where the creation of the request is simply created through an extension method on the protocol itself. Adding all these types and complexity and then not using the benefits of it is a bit of a shame.
To remove the nesting and additional types and protocols, we can introduce a new modelFetcher method:
static func modelFetcher<T, U: Codable>(
createURLRequest: @escaping (T) throws -> URLRequest,
store: NetworkStore = .urlSession
) -> (T) async -> Result<BaseResponseModel<PaginatedResponseModel<U>>, AppError> {
let networkFetcher = self.networkFetcher(store: store)
let mapper: (Data) throws -> BaseResponseModel<PaginatedResponseModel<U>> = jsonMapper()
let fetcher = self.fetcher(
createURLRequest: createURLRequest,
fetch: { request -> (Data, URLResponse) in
try await networkFetcher(request)
}, mapper: { data -> BaseResponseModel<PaginatedResponseModel<U>> in
try mapper(data)
})
return { params in
await fetcher(params)
}
}
This function aims to provide the same level of composability as the original code, but instead of using protocols and types, it injects behavior directly. (As a side note, you could make this into a struct with a closure instead of just a closure if that makes things easier.)
Then, the creation of the actual fetch request closure becomes simple because the only thing that will change is the creation of the request.
static func characterFetcher(
store: NetworkStore = .urlSession
) -> (CharacterFetchData) async -> Result<BaseResponseModel<PaginatedResponseModel<CharacterModel>>, AppError> {
let createURLRequest = { (data: CharacterFetchData) -> URLRequest in
var urlParams = ["offset": "(data.offset)", "limit": "(APIConstants.defaultLimit)"]
if let searchKey = data.searchKey {
urlParams["nameStartsWith"] = searchKey
}
return try createRequest(
requestType: .GET,
path: "/v1/public/characters",
urlParams: urlParams
)
}
return self.modelFetcher(createURLRequest: createURLRequest)
}
We don’t need to wade through many different files or understand many protocols and types because we can inject the behavior through closures instead. The NetworkStore is responsible for actually sending the data over the network, which is why we pass it into the constructor so we can mock it for tests if desired.
A specific example of using behavior instead of types is shown in how we convert this protocol + type from the original project:
protocol NetworkManager {
func makeRequest(with requestData: RequestProtocol) async throws -> Data
}
class DefaultNetworkManager: NetworkManager {
private let urlSession: URLSession
init(urlSession: URLSession = URLSession.shared) {
self.urlSession = urlSession
}
func makeRequest(with requestData: RequestProtocol) async throws -> Data {
let (data, response) = try await urlSession.data(for: requestData.request())
guard let httpResponse = response as? HTTPURLResponse,
httpResponse.statusCode == 200 else { throw NetworkError.invalidServerResponse }
return data
}
}
Into a simple method:
static func networkFetcher(
store: NetworkStore
) -> (URLRequest) async throws -> (Data, URLResponse) {
{ request in
let (data, response) = try await store.fetchData(request)
guard let httpResponse = response as? HTTPURLResponse,
httpResponse.statusCode == 200 else {
throw NetworkError.invalidServerResponse
}
return (data, response)
}
}
You can tell these are almost the same exact code and do the same thing, but we dropped both the type and the protocol and could accomplish the same thing without them.
Another simple example is the jsonMapper function. We just create a JSON mapper and return it as a closure, thus keeping all of the flexibility of the DataParser protocol but without the protocol.
static func jsonMapper<T: Decodable>() -> (Data) throws -> T {
let decoder = JSONDecoder()
decoder.keyDecodingStrategy = .convertFromSnakeCase
return { data in
try decoder.decode(T.self, from: data)
}
}
I think the Network layer becomes much simpler and more direct with this style of composition instead of the protocol/type-based approach.
This isn’t to say you should never use protocols, but think about what the protocol and the type are doing and ask if you need an entire type for every 2–3 lines of code you are going to write.
Project Modularization
Overall, the app is pretty well split up! However, I think the project could benefit by explicitly modularizing the Network module. Think about it — does the composition of our app really need to know which JSON mapper the networking feature is going to use? Can we even change the JSON mapper for the networking feature without breaking it? It would be nice if the Network module handled all of this for us so we can stick to what we are using it for: fetching superheroes.
Let’s limit the Network module to take in whatever we can meaningfully change, like the NetworkStore we pass in for testing purposes, but nothing else. Also, we can have it only expose things we actually use, like the public fetchers themselves, instead of exposing all of the underlying features of the module.
Furthermore, the Network module doesn’t need to know about the domain at all, and it would be nice to remove the ArkanaKeys dependency away from the project and into the Network module since that is the only place it is used. Having a totally isolated Network module would allow us to easily reuse all the networking logic in any app we make with Marvel superheroes.
In my example code, I have done a ‘virtual modularization’ — I haven’t actually created a separate framework for the Network module and moved the ArkanaKeys in dependency in there. Instead, I created a folder and added the access control, which simulates what it would be like if this were a totally separate framework. This is to make it simpler for the demo project, but you can just create a framework and add it to the project to accomplish this goal.
Another lofty goal would be to separate the UI and presentation logic, but they are pretty coupled at this point, and I think that’s OK. I removed the Presentation folder and moved them in with the UI because, at this point, it’s hard to imagine using a HomeViewModel for something besides a HomeView, but this is a matter of organizational taste.
I ended up using a simple Container class instead of Swinject, but that is a matter of taste as well. In any case, the resolver/container should avoid trying to resolve so many specific Network types like NetworkManager, DataSource, Repositories, and the UseCases. In this case, let’s inject the NetworkStore (my replacement for NetworkManager) and resolve the UseCase dependencies directly.
Minor Updates to the UI Layer
I wanted to mention some minor updates to the UI layer that would increase readability and performance by reducing indentation and removing theAnyView type. Extracting View s out of body for readability helps, in my opinion, try to reduce the indentation to only a few levels if possible. The original app goes all the way to 13 levels of indentation in the HomeView which is quite a lot! Also, it is the app’s root, so having this be as readable as possible from the start is a good idea. We can get the indentation down to just five levels pretty easily by extracting the homeView into a computed property. Here’s what that looks like::
public var body: some View {
NavigationStack {
ZStack {
BaseStateView(
viewModel: viewModel,
successView: homeView,
emptyView: BaseStateDefaultEmptyView(),
createErrorView: { errorMessage in
BaseStateDefaultErrorView(errorMessage: errorMessage)
},
loadingView: BaseStateDefaultLoadingView()
)
}
}
.task {
await viewModel.loadCharacters()
}
}
The last thing I wanted to mention was that the app uses a BaseStateView which takes four different AnyView representing views for the different states of the application, like success, empty, error, etc.BaseStateView would feel more at home by using generics instead ofAnyView which doesn’t always perform well for SwiftUI. This will increase performance, but a downside is that it makes us pass in exactly which View s we want for success/empty/create/loading instead of having them done for us automatically in the constructor.
struct BaseStateView<S: View, EM: View, ER: View, L: View>: View {
@ObservedObject var viewModel: ViewModel
let successView: S
let emptyView: EM?
let createErrorView: (_ errorMessage: String?) -> ER?
let loadingView: L?
...
(If the letters are confusing, you can use names instead, likeSuccessView, EmptyView, etc.)
This approach of using one base controller/view feels a bit odd in SwiftUI. It feels more natural to leverage composition and add all of these state handlers as ViewModifiers instead of adding them all directly to a base View . But each has its tradeoffs, and if you really want to force this constructor to remind callers to use them when applicable, this is a decent way to do it (and uses fewer ZStacks)!
struct ErrorStateViewModifier<ErrorView: View>: ViewModifier {
@ObservedObject var viewModel: ViewModel
let errorView: (String) -> ErrorView
func body(content: Content) -> some View {
ZStack {
content
if case .error(let message) = viewModel.state {
errorView(message)
}
}
}
}
Conclusion
Thanks so much to mohaned_y98 for the inspiration and wonderful sample project! This article will look at applying clean architecture principles through a different style than the original project. The original project’s architecture has pros and cons compared to my rewrite, and depending on the needs of your project, you might prefer those tradeoffs.
I tried to refactor the project to keep the spirit of the initial design alive but afforded more ergonomics and readability. I didn’t spend much time on the UI or Presentation layers because they were pretty solid already: maybe I would program them differently from scratch, but they are coded well and work fine.
It would be fun to go over all the unit tests in another article because this one is getting plenty long as it is!
Last thing: it’s always easier to critique someone else’s work instead of your own. What did I miss in my sample project? What would you do differently? Leave a comment below! Thanks for reading!
Sample Projects
Click the picture to check out the original post on the Swift subreddit:
Clean Code Review: Removing All the Extra Types was originally published in Better Programming on Medium, where people are continuing the conversation by highlighting and responding to this story.