Swiftui Code Review

SwiftUI Code Review

Quick Reference

Gates (review workflow)

Complete in order; do not skip ahead.

1. Anchor scope — Pass when: every reviewed file is listed as a repo-relative .swift path (or the review explicitly states “none opened / N/A” with reason). 2. Reference before critique — Pass when: for any non-trivial body, modifier chain, or wrapper-ownership question, you have opened the matching references/*.md row from the table above or you state “not needed” with one line why. 3. Evidence-bound findings — Pass when: each substantive issue includes `[FILE:LINE]` (or a bounded line range) before recommendations; symbols/snippets may supplement but not replace the location anchor; no finding that rests only on “typical SwiftUI” without pointing at this code.

Review Checklist

• [ ] View body under 10 composed elements (extract subviews)
• [ ] Modifiers in correct order (padding before background)
• [ ] @StateObject for view-owned objects, @ObservedObject for passed objects
• [ ] @Bindable used for two-way bindings to @Observable (iOS 17+)
• [ ] LazyVStack/LazyHStack for scrolling lists with 50+ items
• [ ] No AnyView (use @ViewBuilder or generics instead)
• [ ] ForEach uses stable Identifiable IDs (not array indices)
• [ ] All images/icons have accessibilityLabel
• [ ] Custom controls have accessibilityAddTraits(.isButton)
• [ ] Dynamic Type supported (no fixed font sizes)
• [ ] .task modifier for async work (not onAppear + Task)

When to Load References

• Complex view bodies or modifier chains -> view-composition.md
• Property wrapper usage (@State, @Observable) -> state-management.md
• List performance or view identity issues -> performance.md
• VoiceOver or accessibility implementation -> accessibility.md

Review Questions

1. Could this large view body be split into smaller, reusable Views? 2. Is modifier order intentional? (padding -> background -> frame) 3. Is @StateObject/@ObservedObject usage correct for ownership? 4. Could LazyVStack improve this ScrollView's performance? 5. Would VoiceOver users understand this interface?

Accessibility

Accessibility Labels

All interactive elements need descriptive labels.

// BAD - VoiceOver says "heart"
Button(action: { addToFavorites() }) {
    Image(systemName: "heart")
}

// GOOD - VoiceOver says "Add to favorites"
Button(action: { addToFavorites() }) {
    Image(systemName: "heart")
}
.accessibilityLabel("Add to favorites")
.accessibilityHint("Double tap to add to your favorites")

Decorative Images

Hide non-informative images from VoiceOver.

// BAD - VoiceOver reads "star fill"
HStack {
    Image(systemName: "star.fill")
    Text("Premium Feature")
}

// GOOD - decorative image hidden
HStack {
    Image(decorative: "star.fill")
    Text("Premium Feature")
}

// Alternative
Image(systemName: "star.fill")
    .accessibilityHidden(true)

Custom Control Traits

Custom interactive views need accessibility traits.

// BAD - VoiceOver doesn't know it's tappable
struct CustomCheckbox: View {
    @Binding var isChecked: Bool
    var body: some View {
        Image(systemName: isChecked ? "checkmark.square" : "square")
            .onTapGesture { isChecked.toggle() }
    }
}

// GOOD - proper accessibility
struct CustomCheckbox: View {
    @Binding var isChecked: Bool
    var body: some View {
        Image(systemName: isChecked ? "checkmark.square" : "square")
            .onTapGesture { isChecked.toggle() }
            .accessibilityLabel("Agreement")
            .accessibilityAddTraits(.isButton)
            .accessibilityAddTraits(isChecked ? .isSelected : [])
            .accessibilityValue(isChecked ? "Checked" : "Unchecked")
    }
}

Common Traits

Grouping Elements

Combine related elements for VoiceOver.

// BAD - read separately
VStack {
    Text("John Doe")
    Text("Senior Developer")
    Text("San Francisco")
}

// GOOD - single announcement
VStack {
    Text("John Doe")
    Text("Senior Developer")
    Text("San Francisco")
}
.accessibilityElement(children: .combine)

Dynamic Type

Use semantic fonts, not fixed sizes.

// BAD - ignores user preference
Text("Settings")
    .font(.system(size: 17))

// GOOD - scales with preference
Text("Settings")
    .font(.body)

Environment Properties

Check accessibility settings.

@Environment(\.accessibilityReduceMotion) var reduceMotion
@Environment(\.accessibilityDifferentiateWithoutColor) var noColor
@Environment(\.dynamicTypeSize) var typeSize

Critical Anti-Patterns

Review Questions

1. Do all interactive elements have accessibilityLabel? 2. Are decorative images hidden from VoiceOver? 3. Do custom controls have proper accessibility traits? 4. Are related UI elements grouped for VoiceOver? 5. Does the UI support Dynamic Type (no fixed font sizes)? 6. Is color paired with other indicators (shape, text, icon)?

Performance

Lazy Stacks

Use LazyVStack/LazyHStack for scrolling content with many items.

// BAD - all 1000 items rendered immediately
ScrollView {
    VStack {
        ForEach(items) { ItemView(item: $0) }
    }
}

// GOOD - only visible items rendered
ScrollView {
    LazyVStack {
        ForEach(items) { ItemView(item: $0) }
    }
}

AnyView Avoidance

AnyView defeats SwiftUI's type-based diffing.

// BAD - SwiftUI can't diff
func makeView(type: ViewType) -> some View {
    switch type {
    case .a: return AnyView(ViewA())
    case .b: return AnyView(ViewB())
    }
}

// GOOD - preserves type information
@ViewBuilder
func makeView(type: ViewType) -> some View {
    switch type {
    case .a: ViewA()
    case .b: ViewB()
    }
}

ForEach Identity

Use stable Identifiable IDs, never array indices.

// BAD - index changes when array changes
ForEach(items.indices, id: \.self) { index in
    ItemView(item: items[index])
}

// GOOD - stable ID from Identifiable
ForEach(items) { item in
    ItemView(item: item)
}

// BAD - dynamic range without id
@State var count = 5
List(0..<count) { i in Text("Row \(i)") }  // Crashes!

// GOOD - dynamic range with id
List(0..<count, id: \.self) { i in Text("Row \(i)") }

Equatable Views

For complex views, implement Equatable to optimize diffing.

struct CalendarView: View, Equatable {
    let events: [Event]
    let selectedDate: Date

    static func == (lhs: Self, rhs: Self) -> Bool {
        lhs.events.count == rhs.events.count &&
        lhs.selectedDate == rhs.selectedDate
    }

    var body: some View { /* expensive */ }
}

// Usage
ParentView()
    .equatable()

View Body Efficiency

Avoid expensive operations in view body.

// BAD - formatter created on every rebuild
var body: some View {
    let formatter = DateFormatter()
    formatter.dateStyle = .long
    return Text(formatter.string(from: date))
}

// GOOD - cached formatter
private static let formatter: DateFormatter = {
    let f = DateFormatter()
    f.dateStyle = .long
    return f
}()

Expensive Visual Effects

Blur, shadow, and mask cause offscreen rendering.

// CAUTION - expensive in lists
ForEach(items) { item in
    ItemView(item: item)
        .blur(radius: 5)      // Expensive
        .shadow(radius: 10)   // Expensive
}

Critical Anti-Patterns

Review Questions

1. Should this VStack/HStack be Lazy for scrolling performance? 2. Is AnyView used? Can @ViewBuilder replace it? 3. Does ForEach use stable Identifiable IDs? 4. Are expensive computations cached outside view body? 5. Are visual effects (blur, shadow) used sparingly in lists?

State Management

Property Wrapper Quick Reference

@StateObject vs @ObservedObject

Use @StateObject when the view creates and owns the object. Use @ObservedObject when passed from a parent.

// BAD - recreated on every view rebuild
struct ContentView: View {
    @ObservedObject var viewModel = ViewModel()  // Wrong!
}

// GOOD - survives view rebuilds
struct ContentView: View {
    @StateObject var viewModel = ViewModel()
}

// GOOD - received from parent
struct ChildView: View {
    @ObservedObject var viewModel: ViewModel  // Not creating it
}

iOS 17+ @Observable Pattern

With @Observable, use @State at app level, plain properties in children.

@Observable class AppStore { /* ... */ }

@main
struct MyApp: App {
    @State private var store = AppStore()
    var body: some Scene {
        WindowGroup { ContentView(store: store) }
    }
}

// Child receives without wrapper
struct ChildView: View {
    var store: AppStore  // Read-only access
}

// For two-way bindings, use @Bindable
struct EditView: View {
    @Bindable var user: User
    var body: some View {
        TextField("Name", text: $user.name)
    }
}

@State Mistakes

// BAD - @State ignores external updates
struct ChildView: View {
    @State var user: User  // Updates from parent ignored!
}

// BAD - property observers don't work
@State var count = 0 {
    didSet { print("Changed") }  // Never called!
}

// GOOD - use .onChange modifier
.onChange(of: count) { oldValue, newValue in
    print("Changed from \(oldValue) to \(newValue)")
}

Environment Usage (iOS 17+)

// Old syntax
@Environment(\.modelContext) private var context

// New syntax for custom types
@Environment(AuthService.self) private var authService

Critical Anti-Patterns

Review Questions

1. Is @StateObject used when the view creates the object? 2. Is @ObservedObject only used for objects passed from parent? 3. For iOS 17+, is @State with @Observable in a stable parent view? 4. Is @Bindable used where two-way bindings are needed? 5. Is .onChange used instead of didSet on @State?

View Composition

View Body Complexity

Keep view bodies under 10 composed elements. Extract subviews when bodies grow large.

// BAD - massive body
struct ProductView: View {
    let product: Product

    var body: some View {
        VStack {
            // 50+ lines of inline views
        }
    }
}

// GOOD - extracted subviews
struct ProductView: View {
    let product: Product

    var body: some View {
        VStack {
            PriceSection(price: product.price)
            DetailsSection(description: product.description)
        }
    }
}

Computed Property Views

Avoid computed property views - they prevent SwiftUI diffing.

// BAD - can't be diffed
var priceSection: some View {
    VStack { /* content */ }
}

// GOOD - proper View struct enables diffing
struct PriceSection: View {
    let price: Decimal
    var body: some View { /* content */ }
}

Modifier Ordering

Modifiers apply in order. Common patterns:

// BAD - background only covers text
Text("Hello")
    .background(.blue)
    .padding()

// GOOD - background covers padded area
Text("Hello")
    .padding()
    .background(.blue)

// BAD - shadow clipped away
RoundedRectangle()
    .shadow(radius: 5)
    .clipShape(Circle())

// GOOD - shadow visible
RoundedRectangle()
    .clipShape(Circle())
    .shadow(radius: 5)

Subview Parameters

Pass simple parameters, not domain models, for reusability.

// BAD - coupled to model
struct PriceLabel: View {
    let product: Product  // Tightly coupled
}

// GOOD - model-agnostic
struct PriceLabel: View {
    let price: Decimal
    let discount: Decimal?
}

Critical Anti-Patterns

Review Questions

1. Could this view body be split into smaller View structs? 2. Are there computed property views that should be View structs? 3. Is the modifier order intentional and correct? 4. Are subviews model-agnostic and reusable?