unnecessary decoupling of ViewProperties
Closed this issue · 1 comments
pixelzoom commented
The code-review checklist contains these 2 related items:
- Is there any unnecessary coupling? (e.g., by passing large objects to constructors, or exposing unnecessary properties/functions)
- Is there too much unnecessary decoupling? (e.g. by passing all of the properties of an object independently instead of passing the object itself)?
In the case of parameters of type {ViewProperties}
and its subclasses, there's a little too much decoupling in this sim. For example:
class EquationSceneNode extends SceneNode {
/**
* @param {EquationGraph} equationGraph
* @param {BooleanProperty} valuesVisibleProperty
* @param {BooleanProperty} anglesVisibleProperty
* @param {BooleanProperty} gridVisibleProperty
* @param {BooleanProperty} vectorValuesExpandedProperty
* @param {BooleanProperty} equationsExpandedProperty
* @param {BooleanProperty} baseVectorsExpandedProperty
* @param {BooleanProperty} baseVectorsVisibleProperty
* @param {EnumerationProperty.<ComponentStyles>} componentStyleProperty
* @param {number} graphControlPanelBottom
* @param {Object} [options]
*/
constructor( equationGraph,
valuesVisibleProperty,
anglesVisibleProperty,
gridVisibleProperty,
vectorValuesExpandedProperty,
equationsExpandedProperty,
baseVectorsExpandedProperty,
baseVectorsVisibleProperty,
componentStyleProperty,
graphControlPanelBottom,
options ) {
7 of these parameters are the 7 of the 8 fields of EquationViewProperties
. So in this case, it would be better to replace those 7 parameters with one parameter of type EquationViewProperties
.
I'm going to address this case specifically, and others that I identify.
pixelzoom commented
Done, in SceneNode and GraphControlPanel subclasses. Closing.