search

phetsims / number-line-distance

"Number Line: Distance" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 3 forks source link

Observer parameter name doesn't match dependency name in multilink. #52

Closed pixelzoom closed 3 years ago

pixelzoom commented 3 years ago

For code review #35.

In miltilinks, parameter names in the observer function should match the names of the dependencies.

In DistanceShadedNumberLineNode.js

    // Most of the number line's behaviour is handled in this multilink.
    Property.multilink(
      [
        model.distanceLabelsVisibleProperty,
        model.numberLine.displayedRangeProperty,
        model.distanceRepresentationProperty,
        model.isPrimaryControllerSwappedProperty,
        model.numberLine.orientationProperty,
        model.pointValuesProperty,
        model.numberLine.centerPositionProperty // is only necessary for when listeners are shuffled
      ],
      ( distanceLabelsVisible, displayedRange, distanceRepresentation, isPrimaryNodeSwapped, orientation, pointValues ) => {

Parameter isPrimaryNodeSwapped doesn't match dependency model.isPrimaryControllerSwappedProperty. Consider renaming to isPrimaryControllerSwapped.

pixelzoom commented 3 years ago

Same problem in NLDBaseView.js:

    Property.multilink(
      [
        model.distanceRepresentationProperty,
        model.numberLine.orientationProperty,
        model.isPrimaryControllerSwappedProperty,
        model.pointValuesProperty
      ],
      ( distanceRepresentation, orientation, isPrimaryNodeSwapped, pointValues ) => {
SaurabhTotey commented 3 years ago

I fixed the names, so I will close this issue.