Code review в Swift: что проверять и как давать обратную связь

Code review в Swift: что проверять и как давать обратную связь

Code review — это не просто формальная процедура, а мощный инструмент повышения качества кода и профессионального роста команды. Для iOS/macOS разработчиков на Swift грамотно проведённое ревью помогает избежать багов, улучшить архитектуру и поддерживать единый стиль кода. В этой статье мы разберём, на что обращать внимание при code review Swift-проектов, и как давать обратную связь, чтобы она приносила пользу, а не демотивировала.

1. Архитектура и разделение ответственности

Первое, что стоит оценить при ревью — насколько хорошо код соответствует выбранной архитектуре (MVC, MVVM, VIPER, SwiftUI + Combine). Плохая архитектура ведёт к «God-объектам» и сложности поддержки.

Что проверять:

• Не смешивает ли ViewController бизнес-логику и UI-код?
• Вынесена ли сетевая логика в отдельные сервисы (NetworkService)?
• Используются ли протоколы для абстракции зависимостей?
• Не слишком ли много ответственности у одного класса?

// ❌ Плохо: ViewController знает про URLSession и парсинг JSON
class UserViewController: UIViewController {
    func fetchUser() {
        URLSession.shared.dataTask(with: URL(string: "https://api.example.com/user")!) { data, _, _ in
            let user = try? JSONDecoder().decode(User.self, from: data!)
            DispatchQueue.main.async {
                self.updateUI(with: user)
            }
        }.resume()
    }
}

// ✅ Хорошо: логика вынесена в сервис
protocol UserServiceProtocol {
    func fetchUser(completion: @escaping (Result<User, Error>) -> Void)
}

class UserViewModel {
    private let userService: UserServiceProtocol
    
    init(userService: UserServiceProtocol) {
        self.userService = userService
    }
    
    func loadUser() {
        userService.fetchUser { [weak self] result in
            // обработка результата
        }
    }
}

Совет по обратной связи: Вместо «Этот класс слишком большой» напишите: «Я вижу, что UserViewController отвечает и за сетевые запросы, и за UI. Может, вынести запрос в отдельный сервис, как мы обсуждали на ретро? Это упростит тестирование».

2. Безопасность памяти и управление жизненным циклом

Swift — язык с автоматическим подсчётом ссылок (ARC), но это не освобождает разработчика от заботы о циклах сильных ссылок. Ошибки в этой области — частая причина утечек памяти в iOS-приложениях.

Что проверять:

• Используется ли [weak self] или [unowned self] в замыканиях, особенно при работе с async/await и Combine?
• Не создаётся ли циклическая ссылка между делегатом и владельцем?
• Правильно ли освобождаются ресурсы (наблюдатели, таймеры)?

// ❌ Потенциальная утечка: сильная ссылка на self в замыкании
class LocationManager {
    var onLocationUpdate: ((CLLocation) -> Void)?
    
    func startUpdating() {
        someAsyncAPI { location in
            self.onLocationUpdate?(location) // self захвачен сильно
        }
    }
}

// ✅ Исправлено с [weak self]
class LocationManager {
    var onLocationUpdate: ((CLLocation) -> Void)?
    
    func startUpdating() {
        someAsyncAPI { [weak self] location in
            guard let self = self else { return }
            self.onLocationUpdate?(location)
        }
    }
}

Совет по обратной связи: Указывайте не только на проблему, но и на последствия: «Здесь может возникнуть утечка памяти, если LocationManager будет жить дольше, чем нужно. Давай добавим [weak self], чтобы разорвать цикл».

3. Обработка ошибок и опциональность

Swift — строго типизированный язык с мощной системой Optional. Плохая обработка опциональных значений — одна из главных причин крашей (force unwrap). Правильное использование Result, throws и Optional спасает от 90% неожиданных падений.

Что проверять:

• Используется ли guard let или if let вместо force unwrap (!)?
• Обрабатываются ли ошибки в функциях, помеченных throws?
• Используется ли Result для асинхронных операций вместо опциональных completion-блоков?
• Не игнорируются ли ошибки с помощью try? без явной обработки?

// ❌ Опасно: force unwrap и игнорирование ошибки
let data = try! Data(contentsOf: url) // краш, если файл не найден

// ✅ Безопасно: обработка ошибки
do {
    let data = try Data(contentsOf: url)
    // работа с data
} catch {
    print("Не удалось загрузить данные: \(error.localizedDescription)")
    // показать пользователю алерт
}

// ❌ Ошибка: try? без проверки результата
let json = try? JSONSerialization.jsonObject(with: data)
// json будет nil при ошибке, но код продолжит работу

// ✅ Правильно
guard let json = try? JSONSerialization.jsonObject(with: data) else {
    // обработка ошибки
    return
}

Совет по обратной связи: Если видите force unwrap, спросите: «В каком сценарии это значение может быть nil? Если это гарантировано логикой, добавь комментарий. Если нет — лучше использовать guard let с обработкой ошибки».

4. Производительность и стиль кода

На code review важно смотреть не только на корректность, но и на эффективность. Swift-код должен быть не только правильным, но и быстрым, особенно для UI-потока.

Что проверять:

• Не выполняются ли тяжёлые операции на главном потоке (сеть, парсинг JSON, работа с файлами)?
• Используются ли value types (struct, enum) вместо reference types (class) там, где это уместно?
• Нет ли избыточных вычислений в computed properties или замыканиях внутри body во SwiftUI?
• Соответствует ли код принятому в команде Swift Style Guide (именование, форматирование)?

// ❌ Плохо по производительности: вычисление каждый раз
struct UserView: View {
    var user: User
    
    var body: some View {
        Text("\(calculateFullName())") // вызывается при каждом рендере
    }
    
    func calculateFull