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

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

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

1. Архитектура и дизайн: смотрим в корень

Прежде чем погружаться в детали реализации, оцените общую архитектуру решения. Хороший код — это не только синтаксис, но и структура.

• Single Responsibility Principle (SRP): Не пытается ли класс/функция делать слишком много? Например, не смешивает ли ViewController бизнес-логику и сетевые запросы?
• MVVM / MVC / VIPER: Соответствует ли код выбранному паттерну? Если проект использует MVVM, то ViewModel не должна импортировать UIKit.
• Управление зависимостями: Используется ли внедрение зависимостей (DI) или объекты создаются внутри? Жесткие зависимости усложняют тестирование.
• Работа с памятью: Есть ли утечки? Проверьте @escaping замыкания, сильные ссылки на self и возможные циклические захваты.

// Плохо: сильная ссылка на self в замыкании
class NetworkManager {
    var onComplete: (() -> Void)?

    func fetchData() {
        URLSession.shared.dataTask(with: URL(string: "https://api.example.com")!) { data, _, _ in
            self.onComplete?() // Утечка! Замыкание держит сильную ссылку на self
        }.resume()
    }
}

// Хорошо: используем [weak self]
class NetworkManager {
    var onComplete: (() -> Void)?

    func fetchData() {
        URLSession.shared.dataTask(with: URL(string: "https://api.example.com")!) { [weak self] data, _, _ in
            DispatchQueue.main.async {
                self?.onComplete?()
            }
        }.resume()
    }
}

2. Безопасность типов и опциональность: используем мощь Swift

Swift — язык со строгой типизацией, и code review должен это отражать. Избегайте "кода на ObjC в обертке Swift".

• Force unwrapping: Есть ли ! там, где его можно избежать? Используйте guard let или if let.
• Перечисления (Enums): Используются ли они вместо строковых констант? Например, для статусов ответа или типов ошибок.
• Result type: Обрабатывается ли ошибка через Result<Success, Failure> вместо возврата опционального значения и отдельной ошибки?
• Generic: Насколько уместно использование обобщений? Иногда они усложняют код без необходимости.

// Плохо: force unwrap и строковые константы
func getUser(id: String!) -> User? {
    if id == "admin" {
        return User(name: "Admin")
    }
    return nil
}

// Хорошо: enum и безопасное извлечение
enum UserType: String {
    case admin
    case regular
}

func getUser(id: String?) -> User? {
    guard let userId = id, let userType = UserType(rawValue: userId) else {
        return nil
    }
    // ... логика
    return User(name: userType.rawValue)
}

3. Производительность и работа с UI: бережем батарею и FPS

Для iOS/macOS приложений критичны плавность анимаций и энергоэффективность. Обращайте внимание на:

• Main Thread: Все обновления UI должны происходить на главном потоке. Проверьте, нет ли вызовов UIView или NSView из фоновых очередей.
• Тяжелые вычисления: Не выполняются ли сложные операции на главном потоке? Используйте DispatchQueue.global().
• Layout: Не вызывается ли layoutIfNeeded() или setNeedsLayout() слишком часто? Это может привести к лишним перерисовкам.
• Изображения: Используется ли кэширование (например, NSCache или библиотеки типа Kingfisher)? Не загружается ли изображение большего размера, чем нужно для UIImageView.

// Плохо: обновление UI из фонового потока
DispatchQueue.global().async {
    let image = self.downloadImage()
    self.imageView.image = image // КРАШ! UI обновляется не на main thread
}

// Хорошо: явный переход на main
DispatchQueue.global().async {
    let image = self.downloadImage()
    DispatchQueue.main.async {
        self.imageView.image = image
    }
}

4. Как давать обратную связь: искусство конструктивного ревью

Техническая часть — это половина дела. Вторая половина — как вы преподносите свои замечания. Плохая обратная связь может демотивировать разработчика.

• Будьте конкретны: Вместо "этот код плохой" скажите "здесь может возникнуть утечка памяти из-за сильной ссылки на self в замыкании".
• Объясняйте "почему": Укажите не только на проблему, но и на причину. "Это может привести к падению приложения, если сервер вернет nil".
• Предлагайте альтернативы: Не просто критикуйте, а предложите, как исправить. "Давай заменим force unwrap на guard let, это безопаснее".
• Хвалите хорошее: Отмечайте удачные решения. "Отличная идея использовать enum для статусов, код стал читаемее".
• Разделяйте факты и мнения: "Это нарушает style guide" — факт. "Мне кажется, этот метод слишком длинный" — мнение. Мнения стоит обсуждать, а не требовать.

// Пример плохого комментария:
// "Ты что, забыл про weak self? Исправляй!"

// Пример хорошего комментария:
// "Привет! Заметил, что в замыкании dataTask используется сильная ссылка на self.
// Если контроллер будет закрыт до завершения запроса, это приведет