incorrect type expressions for type-specific Property subclasses

[pixelzoom]

Noted in code review #259.

Using {Property.<boolean>}, {Property.<number>}, and {Property.<string>} is generally:

(1) ... unnecessary. In these cases you should be (and are) using BooleanProperty, NumberProperty, and StringProperty respectively.

(2) ... incorrect, because the Property subclasses add functionality to Property. E.g.:

      // @public {Property.<number>} dimensions of the square aperture
      this.squareWidthProperty = new NumberProperty( 16 )

... is incorrect, because NumberProperty has more functionality than {Property.<number>} .

[samreid]

Proposed fix pushed, please review.

[pixelzoom]

Type expressions are also unnecessary in cases like this:

      // @public {BooleanProperty} - whether the wave area graph should be displayed
      this.showGraphProperty = new BooleanProperty( false );

.. for the same reason that they are unnecessary for:

      // @private
      this.eventTimer = new EventTimer( eventTimerModel, timeElapsed =>
        this.advanceTime( 1 / EVENT_RATE, false )
      );

... it's obvious what the type is.

[samreid]

Should I omit {number}?

      // @public {number} phase of the wave generator
      this.phase = 0;

What about Property enums?

// @public {Property.<PlaySpeedEnum>} - the speed at which the simulation is playing
this.playSpeedProperty = new Property( PlaySpeedEnum.NORMAL, {
  validValues: PlaySpeedEnum.VALUES
} );

What about parametric Property instances?

// @public {Property.<Scene>} - selected scene
this.sceneProperty = new Property( this.waterScene, {
  validValues: this.scenes
} );

[samreid]

I removed more cases where the type is obvious, wasn't sure about the cases mention in the preceding comment. @pixelzoom can you please review?

[pixelzoom]

The rule of thumb that I use is "include type expression unless the type is obviously identified in the definition". And I think this is something to keep in mind for the future, not to change retroactively. It doesn't hurt anything to have redundant type expressions. That said...

Should I omit {number}?

      // @public {number} phase of the wave generator
      this.phase = 0;

I think it's fine to omit {number} if the only value it can take is a number. It also doesn't hurt to have the type expression for emphasis, especially when something is not "read-only".

What about Property enums?

// @public {Property.<PlaySpeedEnum>} - the speed at which the simulation is playing
this.playSpeedProperty = new Property( PlaySpeedEnum.NORMAL, {
  validValues: PlaySpeedEnum.VALUES
} );

The type of the Property's value is identified in the definition, it's obviously PlaySpeedEnum. So I don't feel like the type expression is needed.

What about parametric Property instances?

// @public {Property.<Scene>} - selected scene
this.sceneProperty = new Property( this.waterScene, {
  validValues: this.scenes
} );

The type of the Property's value is NOT identified in the definition. You'd have to look elsewhere (at the definitions of this.waterScene and this.scenes) to discover that the type it Scene. So the type expression is useful here.

[samreid]

Thanks, I used the aforementioned rules of thumb to prune redundant annotations further. Ready for review.

[pixelzoom]

👍 Closing.