Why did I code like this?
Greetings to everyone. I often refactor my own projects and see what I can change. This time I was doing this at Socia, which was recently a guest on Chain Of Responsibility. But some things caught my attention!
Why?
This word is truly magical. Why did I do this?
protocol SociaHandler {
associatedtype Request: FirebaseRequestStrategy
var nextHandler: (any SociaHandler)? { get set }
var request: Request? { get set }
init(with handler: (any SociaHandler)?)
func handle(_ onSuccess: (() -> Void)?) async -> LocalizedError?
}- Why is Request a generic type and nextHandler not? (Reason is here)
- Why am I returning LocalizedError?
- Why do I need a closure called onSuccess?
guard model.username.isEmpty == false else { return AuthError.emptyUsername }- Why am I checking for a boolean value? (It made me feel awful)
But this is a really beautiful thing. Because when I look at my code and I can think that something is wrong and I can fix my mistakes, it shows that I can continue my development. And that’s exactly what I want.
Let’s look at the problems together and find our solutions. One thing we need to pay attention to here is that there is something we want to fix. We learned something new, but should we really apply it?
Why is Request a generic type and nextHandler not?
Our first item talks about exactly this. Yes, if we can, it is more performant to get a generic type instead of any keyword. There is a video next to the article in which I explain this issue. (You may say that it might be better if this were done. If you have an idea, please let me know.)
But what I need here is exactly any handler. Because if a chain knew the generic form of the next chain, I would have to change the code so that something could be selected into the chain. And that doesn’t fit Solid’s O. So open closed.
Therefore, this is the answer to our first item. Using generics may cause reasons for me to change the code.
Why am I returning LocalizedError?
The code here is “If an error is received, the error should be returned and I will check it and notify the user.” It is in the form. This isn’t a problem, but why is this a return type?
It would definitely be much better to use throws here. It shouldn’t be your job to debug. If an error occurs, just let the developer know about it. That’s why I updated the code here as follows.
func handle() async throwsNow I throw errors instead of returning them. What did I gain here?
This code performs only one task. In other words, it will do a job and this job may make mistakes. Then I don’t want results. What if I wanted to? Now in this case, I can just add return type and do what is necessary :)
Why do I need a closure called onSuccess?
We can get a clue from the item above. We really don’t need this. Our protocol already has the full new handler.
Old
func handle(_ onSuccess: (() -> Void)?) async -> LocalizedError? {
guard let request else { return AuthError.requestError }
guard request.storageData.isEmpty == false else { return await nextHandler?.handle(nil) }
onSuccess?()
let result: FirebaseResponse<String> = await FirestorageManagerManager.shared.request(request)
if result.status == .success, let downloadLink = result.content {
return await nextHandler?.handle { [weak self] in
guard let handler = self?.nextHandler as? RegisterHandler<RegisterRequest> else { return }
guard var requestModel = handler.request?.model else { return }
requestModel.imagePath = downloadLink
handler.request?.model = requestModel
}
}
return AuthError.uploadError
}New
func handle() async throws {
guard let request else { throw AuthError.requestError }
guard !request.storageData.isEmpty else {
try await nextHandler?.handle()
return
}
try await handleRequestResult(handleRequest(request))
try await nextHandler?.handle()
}
private func handleRequestResult(_ response: FirebaseResponse<String>) throws {
guard response.status == .success, let downloadLink = response.content else {
throw AuthError.uploadError
}
guard let nextHandler = nextHandler as? RegisterHandler else {
throw AuthError.unexpectedError
}
nextHandler.request?.model.imagePath = downloadLink
self.nextHandler = nextHandler
}
private func handleRequest(_ request: UploadProfilePhotoRequest) async -> FirebaseResponse<String> {
await FirestorageManagerManager().request(request)
}The new code is longer. But it’s definitely more understandable. So much so that when I first looked at it, I said, where does the handle method work here?
Logically, when all operations are completed, the new handler should work. But here, even though the transactions are completed, we throw a new job to the other chain. And I think this is not the right approach.
We look at the next handler we have in the new code. And we change its model. Thus, all responsibility for uploading photos remains in this chain.
Why am I checking for a boolean value?
This is entirely my fault. When writing your code, you usually write the easiest thing first, and then you can think about how to write it better. This is how I usually do it. But even though I reviewed my code here, doing long tasks definitely requires a break.
Even if I pay attention to my code while trying to create and explain the article, it seems overlooked and funny. Then let’s immediately update it with the code below and our problem will disappear.
guard !request.storageData.isEmpty else {
try await nextHandler?.handle()
return
}Summary and Closing
If you ask why I wrote this article, it was definitely not to edit the code or share it. I could do this by updating the other article. The main theme here is definitely why we should use the word Why.
Especially when we learn new things, we feel deeply that our code is good and that we are improving ourselves. However, reviewing the code we wrote again (breaks are important here 🙂) gives a good answer to the question of whether we can take it further.
When you do this at regular intervals, you can see that it is improving and be able to criticize your own code. And you can continue to work and produce much more motivated.
Thank you for reading. Have a nice coding :)
