Abstract Class PreProc as interface definition w/o parameter defines

[Bastel Baus]

My first thought:

in abstract class appPreProcessor.py

class PreProc(object, metaclass=ABCPreProcRegister):
There are all preprocessor functions defined but with a generic parameter set. I did not get the intention of defining an abstract class (as interface definition) but not defining the concrete parameters. So nobody who implements a specific preprocessor knows which parameters to implement w/o reading through the whole code. Is there any specific reason for this?

Else I would propose to change the interface (PreProc functions) to concrete parameters (w/ documentation) so that derived classes know the parameters (and also the caller knows what to pass :-)).

OK, for some functions this might be quite some parameters and I understood passing all class paramters might be handsome, but at least if there are specific extra parameter I would propose to name them explicitly.

‌

BR, Bastel Baus

‌

[Marius Stanciu]

Hello Bastel,
The idea is that we have 2 choices (at extremes):
1. You make a very well defined class, with parameters, which will be easy to understand/maintain but can be become hard to read due of the length of the parameters chain.
2. You make a class that is general and which allow easy modification It’s like using the *args, **kwargs parameters. You can always insert a new parameter without making a long chain of parameters.

In the end it’s a choice, neither is best but each approach has its advantages and disadvantages. The guys who implemented chose this way and I followed it since I can’t really change everything. True that over time things might need an update but I was (an in a measure still am) focused in adding new features to the software and not so much in making nice code. But I agree that once we reach a stable form, optimizations should follow, both in the form and speed.
Best regards,
Marius

[Bastel Baus]

Thanks, fully understood your priorization for more features.

And I also understood the two extremes. I think there are some pieces which are not optimal for all options, just as example below; lift_code does provide several different parameters depening on calls throughout the camlib but at least the default preprocessor does only handle the change in z coordinate. So even by reading through the code it is quite difficult to understand the intendet behavor which probably varries at different positions.

camlib.py

3742: t_gcode += self.doformat(p.lift_code, x=self.oldx, y=self.oldy)
3747: t_gcode += self.doformat(p.lift_code, x=0, y=0)  # Move (up) to travel heigh
4050: gcode += self.doformat(p.lift_code, x=self.xy_toolchange[0], y=self.xy_toolchange[1])

preprocessors/default.py

116:    def lift_code(self, p):
117:        return 'G00 Z' + self.coordinate_format % (p.coords_decimals, p.z_move)

‌

So what I understand, if I find the time to do here some improvements, this could be beneficial for the project. Let’s see if I can find some time