Code Review in Swift: What to Check and How to Give Feedback in Practice
\n\nCode review is not just a formality, but a powerful tool for improving code quality, spreading knowledge within the team, and preventing bugs. However, if you approach reviews formally or, conversely, too aggressively, it can do more harm than good. In this article, we will break down what exactly to pay attention to when reviewing Swift code for iOS and macOS, and how to formulate feedback so that it is useful and does not hurt colleagues.
\n\n1. Architecture and Design: Look at the Root
\n\nBefore diving into implementation details, evaluate the overall architecture of the solution. Good code is not just about syntax, but also about structure.
\n\n- \n
- Single Responsibility Principle (SRP): Is the class/function trying to do too much? For example, is the ViewController mixing business logic and network requests? \n
- MVVM / MVC / VIPER: Does the code conform to the chosen pattern? If the project uses MVVM, the ViewModel should not import
UIKit. \n - Dependency Management: Is dependency injection (DI) used, or are objects created internally? Tight dependencies make testing harder. \n
- Memory Management: Are there any leaks? Check
@escapingclosures, strong references toself, and possible retain cycles. \n
// Bad: strong reference to self in a closure\nclass NetworkManager {\n var onComplete: (() -> Void)?\n\n func fetchData() {\n URLSession.shared.dataTask(with: URL(string: "https://api.example.com")!) { data, _, _ in\n self.onComplete?() // Leak! The closure holds a strong reference to self\n }.resume()\n }\n}\n\n// Good: use [weak self]\nclass NetworkManager {\n var onComplete: (() -> Void)?\n\n func fetchData() {\n URLSession.shared.dataTask(with: URL(string: "https://api.example.com")!) { [weak self] data, _, _ in\n DispatchQueue.main.async {\n self?.onComplete?()\n }\n }.resume()\n }\n}\n\n2. Type Safety and Optionals: Leverage the Power of Swift
\n\nSwift is a strongly typed language, and code review should reflect that. Avoid "ObjC code wrapped in Swift".
\n\n- \n
- Force unwrapping: Are there
!where it can be avoided? Useguard letorif let. \n - Enums: Are they used instead of string constants? For example, for response statuses or error types. \n
- Result type: Is the error handled via
Result<Success, Failure>instead of returning an optional value and a separate error? \n - Generics: How appropriate is the use of generics? Sometimes they complicate the code unnecessarily. \n
// Bad: force unwrap and string constants\nfunc getUser(id: String!) -> User? {\n if id == "admin" {\n return User(name: "Admin")\n }\n return nil\n}\n\n// Good: enum and safe unwrapping\nenum UserType: String {\n case admin\n case regular\n}\n\nfunc getUser(id: String?) -> User? {\n guard let userId = id, let userType = UserType(rawValue: userId) else {\n return nil\n }\n // ... logic\n return User(name: userType.rawValue)\n}\n\n3. Performance and UI Work: Save Battery and FPS
\n\nFor iOS/macOS applications, smooth animations and energy efficiency are critical. Pay attention to:
\n\n- \n
- Main Thread: All UI updates must happen on the main thread. Check if there are any calls to
UIVieworNSViewfrom background queues. \n - Heavy Computations: Are complex operations being performed on the main thread? Use background queues for heavy tasks. \n