iOS SDK Code Review

Perform comprehensive code reviews for Payoo iOS Frameworks following Clean Architecture, MVVM, and iOS SDK best practices.

When to Activate

"review code", "code review", "check this code"
"review PR", "review pull request", "review MR"
"check iOS code", "review Swift code"
User asks about code quality or best practices
Reviewing ViewModels, ViewControllers, UseCases, or DataSources

Review Process

1. Identify Code Context

Determine what's being reviewed:

File type: ViewModel, ViewController, UseCase, DataSource, Model, Service
Framework: PayooCore, PayooEwallet, PayooPayment, etc.
Layer: Presentation (Scenes), Domain (UseCase), Data (DataSources/Services)

2. Architecture Review

Clean Architecture Compliance:

✓ Proper layer separation (Presentation/Domain/Data)
✓ Dependencies point inward (Presentation → Domain → Data)
✓ ViewModels don't directly access Services (must use UseCases)
✓ Models in correct layer (Domain models vs Data models)

MVVM Pattern:

✓ ViewModel has protocol definition ({Feature}ViewModelType)
✓ Delegate protocol exists ({Feature}ViewModelDelegate)
✓ ViewController implements delegate
✓ ViewModel is testable (no UIKit dependencies)
✓ View state managed through delegate callbacks

UseCase Pattern:

✓ Business logic in UseCases, not ViewModels
✓ Single responsibility per UseCase
✓ UseCases injected into ViewModels
✓ UseCases coordinate repositories/services

3. Memory Management Review

Retain Cycles:

✓ Delegates marked weak
✓ Closures use [weak self] or [unowned self] appropriately
✓ No strong reference cycles in ViewModels
✓ Timer/observer cleanup in deinit

Example issues:

// ❌ BAD: Strong delegate reference
var delegate: SomeDelegate?

// ✅ GOOD: Weak delegate reference
weak var delegate: SomeDelegate?

// ❌ BAD: Strong self in closure
viewModel.loadData { data in
    self.updateUI(data)
}

// ✅ GOOD: Weak self in closure
viewModel.loadData { [weak self] data in
    self?.updateUI(data)
}

4. Dependency Injection Review

Constructor Injection:

✓ Dependencies injected via init
✓ All required dependencies in initializer
✓ No service locator or singletons (except context)
✓ Dependencies are protocols, not concrete types

Example:

// ✅ GOOD: Constructor injection
final class DepositViewModel {
    private let depositAmountUC: DepositAmountUseCase
    private let bankAccountUC: BankAccountUseCase

    init(depositAmountUC: DepositAmountUseCase,
         bankAccountUC: BankAccountUseCase) {
        self.depositAmountUC = depositAmountUC
        self.bankAccountUC = bankAccountUC
    }
}

// ❌ BAD: Service locator pattern
let service = ServiceLocator.shared.depositService

5. Naming Conventions Review

File Names:

✓ ViewModels: {Feature}ViewModel.swift
✓ ViewControllers: {Feature}ViewController.swift
✓ UseCases: {Feature}UseCase.swift
✓ DataSources: {Feature}DataSource.swift
✓ Cells: {Name}Cell.swift

Class/Protocol Names:

✓ Protocols end with Type for interfaces: DepositViewModelType
✓ Delegate protocols end with Delegate: DepositViewModelDelegate
✓ Clear, descriptive names (no abbreviations)
✓ Consistent with project conventions

Variables:

✓ context for PayooEwalletContext
✓ {name}UC for UseCase instances: depositAmountUC
✓ Descriptive names, avoid single letters (except in loops)

6. API Design Review

For SDK Public APIs:

✓ Clear, self-documenting method names
✓ Delegate patterns for callbacks
✓ Error handling with proper Error types
✓ Thread-safe if needed
✓ No force unwrapping in public APIs
✓ Proper access control (public, internal, private)

Error Handling:

// ✅ GOOD: Proper error handling
enum DepositError: Error, LocalizedError {
    case outOfRange(bank: String, min: Double, max: Double)

    var errorDescription: String? {
        switch self {
        case .outOfRange(let bank, let min, let max):
            return "Amount out of range for \(bank): \(min)-\(max)"
        }
    }
}

// ❌ BAD: Generic errors
throw NSError(domain: "Error", code: -1, userInfo: nil)

7. Swift Best Practices

Code Quality:

✓ No force unwrapping (!) unless absolutely safe
✓ Use guard let for early returns
✓ Prefer let over var
✓ Access control appropriately set
✓ No commented-out code
✓ Proper use of final for classes not meant to be subclassed

SwiftLint Compliance:

✓ Line length ≤ 120 characters
✓ File length ≤ 500 lines (warning), ≤ 1200 (error)
✓ Type body length ≤ 300 lines (warning), ≤ 400 (error)
✓ No trailing whitespace

8. Multi-Target Configuration

Internal/External Builds:

✓ Internal-only code wrapped in #if INTERNAL
✓ No internal features leaking to external builds
✓ Proper preprocessor flag usage

#if INTERNAL
    // Internal-only features
    func debugFunction() { }
#endif

9. Localization Review

✓ All user-facing strings use L10n.* (SwiftGen)
✓ No hardcoded strings for UI text
✓ Proper format strings for dynamic content

// ✅ GOOD: Localized strings
let title = L10n.Deposit.Navigation.deposit
let message = L10n.Message.Deposit.outOfRange(min, max, bank)

// ❌ BAD: Hardcoded strings
let title = "Deposit"

Output Format

Provide review as structured report:

## Code Review: {FileName}

### ✅ Strengths
- [List what's done well]

### ⚠️ Issues Found

#### 🔴 Critical Issues
**Issue:** [Description]
**Location:** {File}:{Line}
**Impact:** [Why this matters]
**Fix:**
\```swift
// Current code
[problematic code]

// Suggested fix
[fixed code]
\```

#### 🟡 Warnings
**Issue:** [Description]
**Location:** {File}:{Line}
**Suggestion:** [How to improve]

#### 🔵 Suggestions
**Enhancement:** [Description]
**Benefit:** [Why this would help]

### 📊 Summary
- Critical Issues: X
- Warnings: Y
- Suggestions: Z
- Overall: [Pass/Needs Work/Fail]

### 🎯 Priority Actions
1. [Most important fix]
2. [Second priority]
3. [Third priority]

Review Checklists by File Type

ViewModel Review

[ ] Has protocol definition ({Name}ViewModelType)
[ ] Has delegate protocol ({Name}ViewModelDelegate)
[ ] Delegate marked weak
[ ] Dependencies injected via init
[ ] No UIKit imports
[ ] Uses UseCases for business logic
[ ] Closures use [weak self]
[ ] Testable (no side effects in init)

ViewController Review

[ ] Inherits from appropriate base class
[ ] Implements ViewModel delegate
[ ] Sets up analytics (analyticsFeature, analyticsScreenName)
[ ] Proper lifecycle management
[ ] IBOutlets are weak
[ ] Navigation setup in viewDidLoad or dedicated method
[ ] No business logic (delegated to ViewModel)

UseCase Review

[ ] Single responsibility
[ ] Injected dependencies
[ ] No UIKit dependencies
[ ] Proper error handling with typed errors
[ ] Testable

DataSource Review

[ ] Conforms to UITableView/UICollectionView protocols
[ ] Clean separation from ViewController
[ ] Reusable cell registration
[ ] Proper indexPath handling

Key Principles

Clean Architecture First: Verify proper layer separation
Memory Safety: Check for retain cycles, weak references
Dependency Injection: Ensure dependencies are injected, not created
Naming Consistency: Follow project naming conventions
SDK Quality: Public APIs are well-designed and documented
Testability: Code is structured for unit testing
Swift Idioms: Use modern Swift patterns

Quick Commands

If you need to review specific patterns across the codebase:

# Find all ViewModels
grep -r "class.*ViewModel" --include="*.swift" PayooEwallet/

# Find potential retain cycles (strong self in closures)
grep -r "{ self\." --include="*.swift" PayooEwallet/

# Find force unwraps
grep -r "!" --include="*.swift" PayooEwallet/ | grep -v "!="

# Find hardcoded strings
grep -r "\"[A-Z]" --include="*.swift" PayooEwallet/ | grep -v "L10n"