-
Notifications
You must be signed in to change notification settings - Fork 5
Description
Right now the modifier class has an axis parameter that determines the axis of position and dimension modifications.
This parameter should instead be part of the modification itself, to have a better separation of concerns, increase code cohesion, and make the library easier to use.
For example, in the FixedOffsetModifier class there is a function that shifts the origin of the links and joints in the three dimensions. Since this is a method of the modifier class itself we shouldn't create more modifiers inside it, so it works like this:
original_modifier_axis = modifier.axis # save original modifier axis
modification = Modification()
modifier.axis = Side.X # change it for each axis you want to change
modification.add_position(value_x, absolute)
modifier.modify(modification)
modifier.axis = Side.Y
modification.add_position(value_y, absolute)
modifier.modify(modification)
modifier.axis = Side.Z
modification.add_position(value_z, absolute)
modifier.modify(modification)
modifier.axis = original_modifier_axis # restore original modifier axisThis is how it would look with the axis being in the modification:
modification = Modification()
modification.axis = Side.X
modification.add_position(value_x, absolute)
modifier.modify(modification)
modification.axis = Side.Y
modification.add_position(value_y, absolute)
modifier.modify(modification)
modification.axis = Side.Z
modification.add_position(value_z, absolute)
modifier.modify(modification)The code is shorter and makes more intuitive sense.
This change would break existing code however, so I'm open to feedback on the idea @traversaro @CarlottaSartore @AlexAntn
Dod
The axis parameter belongs to the Modification class instead of Modifier