IOS Sdk Code Review

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

1. Clean Architecture First: Verify proper layer separation
2. Memory Safety: Check for retain cycles, weak references
3. Dependency Injection: Ensure dependencies are injected, not created
4. Naming Consistency: Follow project naming conventions
5. SDK Quality: Public APIs are well-designed and documented
6. Testability: Code is structured for unit testing
7. 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"