Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persist rules: Use Repository Pattern to decouple implementation #2675

Open
MattHag opened this issue Nov 10, 2024 · 0 comments · May be fixed by #2678
Open

Persist rules: Use Repository Pattern to decouple implementation #2675

MattHag opened this issue Nov 10, 2024 · 0 comments · May be fixed by #2678

Comments

@MattHag
Copy link
Collaborator

MattHag commented Nov 10, 2024

Is your feature request related to a problem? Please describe.
The rule persistence implementation is tightly coupled with the remaining code and is hard to replace for testing. Also refactoring is prone to errors due to its tight coupling with the remaining system.

Describe the solution you'd like
Apply the Repository Pattern to avoid direct dependency on the rule storage implementation. Instead, pass the dependent (dependency inversion principle), so any implementation can be used at runtime and only needs to satisfy the interface.

Advantages

  • The rule storage implementation is easy to replace with a fake for tests.
  • A distinct interface makes it easy to replace a component
  • Hard coupling of Solaar core and the rule persistence implementation becomes loosely.

Additional context
This leads our way to a layered architecture, which is easy to maintain and well to test.

Information

  • Solaar version: 1.1.13
@MattHag MattHag changed the title Persist rules: Introduce Repository Pattern to decouple Persist rules: Use Repository Pattern to decouple implementation from Nov 11, 2024
@MattHag MattHag changed the title Persist rules: Use Repository Pattern to decouple implementation from Persist rules: Use Repository Pattern to decouple implementation Nov 11, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Nov 12, 2024
Introduce a AbstractRepository as interface to the rules database and
implement a YML storage that implements it.

Additionally, a FakeStorage implements that interface to test diversion
related code, without I/O calls and side effects. It keeps the rules
in-memory for the test duration.

Related pwr-Solaar#2675
MattHag added a commit to MattHag/Solaar that referenced this issue Nov 12, 2024
Introduce a higher level storage provider interface to decouple storage
implementation from diversion dialog and use composition to pass an
implementation.

This simplifies testing of the diversion dialog and enables clean unit
testing without I/O.

Related pwr-Solaar#2675
MattHag added a commit to MattHag/Solaar that referenced this issue Nov 12, 2024
Introduce a AbstractRepository as interface to the rules database and
implement a YML storage that implements it.

Additionally, a FakeStorage implements that interface to test diversion
related code, without I/O calls and side effects. It keeps the rules
in-memory for the test duration.

Related pwr-Solaar#2675
MattHag added a commit to MattHag/Solaar that referenced this issue Nov 12, 2024
Introduce a higher level storage provider interface to decouple storage
implementation from diversion dialog and use composition to pass an
implementation.

This simplifies testing of the diversion dialog and enables clean unit
testing without I/O.

Related pwr-Solaar#2675
MattHag added a commit to MattHag/Solaar that referenced this issue Nov 12, 2024
Introduce a higher level storage provider interface to decouple storage
implementation from diversion dialog and use composition to pass an
implementation.

This simplifies testing of the diversion dialog and enables clean unit
testing without I/O.

Related pwr-Solaar#2675
@MattHag MattHag linked a pull request Nov 12, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant