Skip to content

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

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.

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