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.

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

Synchronization is unreadable and error-prone #74

Closed
rmnldwg opened this issue Feb 15, 2024 · 0 comments
Closed

Synchronization is unreadable and error-prone #74

rmnldwg opened this issue Feb 15, 2024 · 0 comments
Assignees
@rmnldwg
Labels
1.0 release Issues to fix before 1.0 release code quality Improvements w.r.t. readability of code & best practices of coding feature New feature or request
Milestone
version 1.0.0

Comments

@rmnldwg
Copy link
Owner

rmnldwg commented Feb 15, 2024

I think my attempt of synchronizing the attributes of the ipsi and contra instances in the Bilateral model class are bad: It's hard to understand what's going on, the state of some objects changes "magically", one cannot even look up what is synchronized, and I am loosing my mind over it.

However, I think there may be simpler and more elegant solution: A SynchronizerMixing, in a similar fashion to the DelegatorMixin: Such a mixin class could dynamically create methods and properties that keep e.g. some ipsi and contra attributes in sync. The advantage would be that the entire synchronization is now the responsibility of the Bilateral (or Midline) class. The ipsi and contra objects would - on their own - still work as expected and not leak anything to each other via obscure synchronization callbacks.

This could be much easier to test, as well.
@rmnldwg rmnldwg added feature New feature or request code quality Improvements w.r.t. readability of code & best practices of coding 1.0 release Issues to fix before 1.0 release labels Feb 15, 2024
@rmnldwg rmnldwg added this to the version 1.0.0 milestone Feb 15, 2024
@rmnldwg rmnldwg self-assigned this Feb 15, 2024
rmnldwg added a commit that referenced this issue Feb 15, 2024
@rmnldwg
feat: add delegator/synchronizer mixin
9676dd9 
Related: #74
rmnldwg added a commit that referenced this issue Feb 16, 2024
@rmnldwg
feat: add delegation/sync mixin
b2ce7e4 
We have added a mixin class that allows the delegation and
synchronization of attributes. This may replace the convoluted and
unreadable callback functions we used to sync some properties

Related: #74
@rmnldwg rmnldwg mentioned this issue Mar 4, 2024
Merged
@rmnldwg rmnldwg closed this as completed in 156ee60 Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rmnldwg rmnldwg

Labels
1.0 release Issues to fix before 1.0 release code quality Improvements w.r.t. readability of code & best practices of coding feature New feature or request
Projects
None yet
Milestone
version 1.0.0
Development

When branches are created from issues, their pull requests are automatically linked.

74-synchronization-is-unreadable-and-error-prone rmnldwg/lymph
1 participant
@rmnldwg