Skip to content

[SPoA/Clean Code] Clarify the SettingsCoordinator snapshot API as read-only #121

Closed

Description

@KeyffMS

Audit finding

SettingsCoordinator.Current correctly returns a defensive deep copy, but its declared type is the mutable domain model SightAdaptSettings. Consumers can change properties and collections on the returned object even though those changes are intentionally discarded unless performed inside Commit.

The behavior is safe, but the API communicates a writable current state rather than a read-only snapshot and makes accidental no-op mutation easy during future maintenance.

Target state

Make the read boundary explicit while preserving the existing transactional mutation authority in SettingsCoordinator.Commit.

Suitable options include:

  • expose IReadOnlySightAdaptSettings Current backed by a defensive snapshot; or
  • replace the property with an explicitly named CreateSnapshot() API and keep mutable settings internal to transaction callbacks.

Acceptance criteria

  • Callers cannot reasonably interpret the read API as a way to update committed settings.
  • Mutable SightAdaptSettings remains available to domain mutation services only inside explicit transaction/normalization boundaries.
  • Existing defensive-copy behavior is preserved.
  • Application use cases accept IReadOnlySightAdaptSettings wherever mutation is not required.
  • Tests verify that a read snapshot cannot affect committed automatic mode, assignments or visual profiles.
  • The selected API naming and ownership are documented in docs/ARCHITECTURE.md.
  • No repository abstraction is introduced.

Completion evidence — 2026-09-24

Implemented in PR #157 and merged as 10e3e64747e8982a116aa4e686ac943ba99eb35c.

Verified pull-request checks:

  • DCO run 36000698085 — success;
  • full Windows build/test/release-compliance run 36000698065 — success.

The implementation exposes read snapshots through IReadOnlySightAdaptSettings, preserves defensive copies, centralizes shared use-case transaction plumbing in SettingsUseCaseContext, and routes both Commit and PersistCurrent through one private persistence execution path while preserving their distinct change-event behavior.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions