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

Refactor: Make transform configurations explicit, rework parsing of transforms #119

Conversation

Beetelbrox
Copy link
Collaborator

@Beetelbrox Beetelbrox commented Jun 7, 2024

Closes: #122

Description

As most entities in the project transforms were storing their configuration in a dictionary defined in RbtParamHandler, several steps higher in the inheritance hierarchy. This made it very difficult to understand which configuration each transform needed, which ones were the default values, and which value was being used at any given point in time.

Proposed changes

To address the above explained we've made the parameters required by each one of the transforms explicit. Now each transform defines a Config struct with their respective configuration and transform constructors now accept this configuration instead of having to set arbitrary parameters passing strings as we did before. The TransformFactory builds transforms by grabbing the necessary values from the ParameterSource internal state and building the appropriate config for each transform, explicitly stating which fields are needed instead of dynamically iterating through everything as we did before

Beetelbrox added 30 commits June 6, 2024 08:46
@Beetelbrox Beetelbrox changed the title Fjurado/refactor make transform member variables explicit Refactor: Make transform configurations explicit, rework parsing of transforms Jun 7, 2024
@Beetelbrox Beetelbrox marked this pull request as ready for review June 7, 2024 10:37
@Beetelbrox Beetelbrox merged commit ea05b80 into CBDD:main Jun 7, 2024
9 checks passed
@Beetelbrox Beetelbrox deleted the fjurado/refactor-make-transform-member-variables-explicit branch June 7, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prepare configuration parsing overhaul
2 participants