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.

Sign up for GitHub

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

Jump to bottom

Settings: Reduce complexity and tight coupling #2659

Open

MattHag opened this issue Nov 3, 2024 · 0 comments

Labels

Collaborator

MattHag commented Nov 3, 2024 •

edited

Loading

The Setting class in logitech_receiver/settings contains dozens of subclasses, which are expected to contain specific properties. However, that's not ensured via implementation and makes it so hard to reafactor and maintain.

A dependency graph for the logitech_receiver module can be created via pyreverse

pyreverse --filter PUB_ONLY --output-directory pyreverse_output --output dot ./lib/logitech_receiver/* && dot -T svg classes.dot -o modeling_full.svg && open modeling_full.svg

The diagram shows an unbelievable amount of tightly coupled classes with multiple inheritance levels. This makes it very hard to test, refactor and improve related code.

The problems are

Complex code with many hard connections, "big ball of mud"
- NamedInts
- No type hints
Hard to maintain with several levels of subclasses
No documentation of available/expected properties nor enforced

Necessary actions
Replace inheritance with composition, e.g. the RuleComponent is only a function. There's no need to inherit 100 classes from it, rather convert it into a function.

Information

Solaar version: 1.1.13

The text was updated successfully, but these errors were encountered:

MattHag added the enhancement label

MattHag changed the title ~~Settings: Reduce complexity and dependencies~~ Settings: Refactor code to reduce complexity and dependencies

MattHag mentioned this issue

Refactor settings and related code #2660

Merged

MattHag mentioned this issue

Aim towards a layered architecture using dependency inversion principle #2676

Open

MattHag changed the title ~~Settings: Refactor code to reduce complexity and dependencies~~ Settings: Reduce complexity and tight coupling

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Avoid tight coupling between Condition and Action

d0a4116

Replace tight coupling of classes by converting their common base class
into a simple function, that is used instead. This change completely
removes the RuleComponent class.

The RuleComponent was a base class solely holding a compile function and
passing it to its children. There is no need for tight coupling with
inheritance for that purpose.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Avoid tight coupling between Condition and Action

b851724

Replace tight coupling of classes by converting their common base class
into a simple function, that is used instead. This change completely
removes the RuleComponent class.

The RuleComponent was a base class solely holding a compile function and
passing it to its children. There is no need for tight coupling with
inheritance for that purpose.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Decouple Action and Condition classes

97b0d0b

Convert the base classes into protocols to decouple all related classes
from each other, while still enforcing a common interface.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Decouple Action and Condition classes

81a9d84

Convert the base classes into protocols to decouple all related classes
from each other, while still enforcing a common interface.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Remove RuleComponent class to avoid coupling of classes

c48d622

Replace tight coupling of Action and Condition classes by removing their
common base class and converting it into a function.

The RuleComponent was a base class solely holding a compile function and
passing it to its children. There is no need for tight coupling with
inheritance for that purpose.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Introduce protocols to unite Action and Condition classes

c0ed6d4

Enforce a common interface for all Action and Condition related classes
and connect them to a common protocol class to support isinstance
checks.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Remove RuleComponent class to avoid coupling of classes

fc25353

Replace tight coupling of Action and Condition classes by removing their
common base class and converting it into a function.

The RuleComponent was a base class solely holding a compile function and
passing it to its children. There is no need for tight coupling with
inheritance for that purpose.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Introduce protocols to unite Action and Condition classes

58119ca

Enforce a common interface for all Action and Condition related classes
and connect them to a common protocol class to support isinstance
checks.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Remove RuleComponent class to avoid coupling of classes

b64c2c4

Replace tight coupling of Action and Condition classes by removing their
common base class and converting it into a function.

The RuleComponent was a base class solely holding a compile function and
passing it to its children. There is no need for tight coupling with
inheritance for that purpose.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Introduce protocols to unite Action and Condition classes

f2b42f2

Enforce a common interface for all Action and Condition related classes
and connect them to a common protocol class to support isinstance
checks.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Remove RuleComponent class to avoid coupling of classes

64f22ce

Replace tight coupling of Action and Condition classes by removing their
common base class and converting it into a function.

The RuleComponent was a base class solely holding a compile function and
passing it to its children. There is no need for tight coupling with
inheritance for that purpose.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Introduce protocols to unite Action and Condition classes

6a99fc6

Enforce a common interface for all Action and Condition related classes
and connect them to a common protocol class to support isinstance
checks.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Remove RuleComponent class to avoid coupling of classes

97972d7

Replace tight coupling of Action and Condition classes by removing their
common base class and converting it into a function.

The RuleComponent was a base class solely holding a compile function and
passing it to its children. There is no need for tight coupling with
inheritance for that purpose.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Introduce protocols to unite Action and Condition classes

9774ed9

Enforce a common interface for all Action and Condition related classes
and connect them to a common protocol class to support isinstance
checks.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Remove RuleComponent class to avoid coupling of classes

9af34b3

Replace tight coupling of Action and Condition classes by removing their
common base class and converting it into a function.

The RuleComponent was a base class solely holding a compile function and
passing it to its children. There is no need for tight coupling with
inheritance for that purpose.

Related pwr-Solaar#2659

MattHag added a commit to MattHag/Solaar that referenced this issue


          diversion: Introduce protocols to unite Action and Condition classes

a70366a

Enforce a common interface for all Action and Condition related classes
and connect them to a common protocol class to support isinstance
checks.

Related pwr-Solaar#2659

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment