Use mixins to organize properties

[TheAssassin]

Right now, properties are quite messy. I started some research on using mixins to clean those up.

My idea is to add separate mixin classes that represent the various properties and can be combined to classes. For instance:

• FloatMaxPowerPropertyMixin
• FloatMinPowerPropertyMixin
• FocusPropertyMixin
• SpeedPropertyMixin
• BottomUpEngravingPropertyMixin

These mixins could be used to create the property classes used by the various drivers, e.g.:

• PowerSpeedFocusProperty uses the base class plus the three property mixins
• EpilogEngravingProperty uses the base class plus the focus, speed and bottom up engraving property mixins

I'd in fact try to remove most of the actual implementations and use driver-specific classes. By having most of the code organized in the mixins, the redundancy between the classes is still greatly reduced.

Another advantage of this is that we will have no more hierarchies between properties. Currently, property classes are stacked up using inheritance to combine various functionality in a single class without having to duplicate code. This doesn't work that well at the moment, e.g., the int and float power classes are redundant.

The major downside is that Java itself cannot do mixins. We'd have to add a dependency on a separate module (there are a few to choose from) to implement such a structure.

I believe that using mixinx is very much in the interest of the project. It reduces the required maintenance efforts, it eliminates all redundant code between the property classes, and it allows driver developers to extend the functionality by just throwing in another mixin into their properties.

To give an example from real life, let's have a look at #219. There, a new focus property is needed in addition to the existing (very much cumbersome and long) FloatMinMaxPowerSpeedFrequencyProperty in order to be able to configure the focus within VisiCut. To add this, code must likely be duplicated from the PowerSpeedFocusFrequencyProperty. This is far from ideal. The current workaround is to just use the latter class and set both min and max power to the desired power.

I am working on a refactoring of the current properties to eliminate redundancies as much as possible while polishing/finishing the implementation of #219. We can add mixins later on.

[mgmax]

Another idea: Would it be possible to replace all existing property classes with AbstractLaserProperty or simple subclasses thereof?

If I see it correctly, then we (almost?) only use LaserProperty as a "dumb" property storage, only with get/set but without special functionality.

Some issues that would need to be handled:

• Loading/saving property files with XStream
• Backwards compatibility with old property files (custom XStream converter?). May already work due to existing fallback functionality. https://github.com/t-oster/VisiCut/blob/f6ca970b13a31e7a965f9f19d72c399fa8735754/src/main/java/de/thomas_oster/visicut/managers/LaserPropertyManager.java#L112-L150
• How to handle changed property range or added/removed property names. For example, when a focus setting is added to the laser driver, then old objects (settings loaded from disk) would not have the property, so you can't set the focus for any material for which you already have settings. Maybe we can use a similar workaround as in the above link.

Example: K40Nano already uses the AbstractLaserProperty but sets a fixed limit for the speed. We can't use such a fixed limit for the Generic GCode driver where the maximum speed is configurable.

LibLaserCut/src/main/java/de/thomas_oster/liblasercut/drivers/K40NanoDriver.java

Lines 291 to 298 in 1de68f3

	public LaserProperty getLaserPropertyForVectorPart()
	{
	AbstractLaserProperty property = new AbstractLaserProperty();
	property.addPropertyRanged("mm per second", 30f, 0.4f, 240f);
	property.addPropertyRanged("power", 1000, 0, 1000);
	property.addPropertyRanged("diagonal speed adjustment", 0.2612f, 0f, 1f);
	return property;
	}

[TheAssassin]

Loading/saving property files with XStream

This should be replaced entirely anyway. I think it's anything but straightforward how the configuration is stored at this point and it's also really hard work to ensure backwards compatibility. I therefore wouldn't be too worried about it at this point. I suggest to use some adapter in between the internal representation (i.e., business logic) and storage. The old classes can be recycled to convert the old configuration. But this is for another issue.

I like your idea. It would make more sense to use the builder pattern, though, to separate the creation of those objects from their use:

AbstractLaserProperty property = (new AbstractLaserPropertyFactory()).addRangedFloat("mm/s", 30f, 0.4f, 240f).build();

That's going to allow us to remove most of the property code. And it won't prevent us from adding some factories in some utility class to avoid too much redundant code.

How to handle changed property range or added/removed property names. For example, when a focus setting is added to the laser driver, then old objects (settings loaded from disk) would not have the property, so you can't set the focus for any material for which you already have settings. Maybe we can use a similar workaround as in the above link.

The most robust way would be to use some sort of versioned configuration files per driver, keeping the old loaders upon changes to ensure backwards compatibility at least for a while. To do so, one needs to add yet another layer of abstraction in between the drivers and the configuration. But that is also a chance to move on from XStream and make the configuration files actually editable instead of having to use the UI all the time.

I am thinking of a abstract class DriverConfiguration that is instantiated as K40NanoDriverConfigurationV1 for instance. Following the idea of semver, every breaking change requires a version bump, i.e., a new class. All sorts of additions or patches can be implemented in the existing class then.