phetsims/vector-addition

unnecessary decoupling of ViewProperties

Closed this issue · 1 comments

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.

Done, in SceneNode and GraphControlPanel subclasses. Closing.