numberscope / frontscope

Numberscope's front end and user interface: responsible for specifying sequences and defining and displaying visualizers
MIT License
7 stars 15 forks source link

name and description properties being used incoherently in class hierarchy #326

Closed gwhitney closed 3 months ago

gwhitney commented 7 months ago

The ParamableInterface defines r/w string properties 'name' and 'description'. Paramable of course implements them, as (data) properties, with very generic default values. Sequence classes (which all descend from Paramable) override them in fairly reasonable ways, and to some extent take advantage of the fact that they are per-instance rather than per-class, in that a specific instance of the Naturals sequence might end up with name either "Positive Integers" or "Nonnegative Integers". However, there is a slight issue that the default name assigned to instances of Sequence types always matches the name separately specified in the SequenceExportModule, so we have a WET (Write Everything Twice) situation. It's particularly slight in that ultimately there are unlikely to be many different sequence classes as we progressively fold them together using formulas.

On the other hand, in the Visualizer hierarchy, Glen forgot this situation in the recent refactor of VisualizerInterface and VisualizerDefault -> P5Visualizer. He merely observed the WETness mentioned above, which was also happening with Visualizers, and the fact that in this hierarchy there were essentially no uses of name or description except in VisualizerExportModule, and that name only depended on class, not instance. Since at the time VisualizerExportModule is called, there is no instance, he changed name into a static property visualizationName so that it would be accessible; and since the Vue framework only has an instance and doesn't know the class, he created a visualization() method to obtain this property from an instance.

These changes now leave the name and description properties unreliable across the tree of descendents of Paramable. We need to either redesign a consistent interface that fills the needs across the full hierarchy, or drop such items from Paramable (if they are never being used through the paramable interface directly) and have separate properties tailored to each of the Sequence and Visualizer hierarchies separately. On the other hand, it might be worthwhile to have instance names for visualizers, too, so we could consider across the board having staticName (i.e., the generic name for the items of a class, like Naturals) that is a static property and instanceName that is an instance property. Then we could use each of those properly for all the existing uses, I believe. We would still have to decide if we want description, and if so what it is used for, and what conditions it should satisfy.

Then we should implement the new properties uniformly across the board and make sure they are well documented so this sort of fragmentation doesn't happen again. What do others think the interface design should be here?

gwhitney commented 5 months ago

This soup is further stirred by #354 in which the name and description values are put in top-level variables of the respective files and then mentioned both in the class definitions and again in the export modules. This approach was used to allow them to be class fields, so that they could be modified by individual classes, and yet still have their default values accessible in the export modules. There is no way, except with .toString() hackery, to access the default (initializer) value of class fields from the class constructor.

However, I still think that the semantics and implementation of these properties is still not quite on the button. It seems that there should be some fixed properties that are likely static class fields (and hence will be accessible from the constructor) and some mutable per-instance properties that allow the information to be customized to the instance, like the "Positive Integers" and "Nonnegative Integers" possibilities for the natural number sequence.

gwhitney commented 3 months ago

I feel this situation is now resolved in ui2. Since I filed this issue, I am taking the liberty of closing it.