more REVIEW comments #259

Signed-off-by: Chris Malley <cmalley@pixelzoom.com>

js/common/model/Scene.js

@@ -141,6 +141,7 @@ define( require => {
       // note: this is a floating point representation in 2D to work seamlessly with DragListener
       // lattice computations using this floating point value should use Math.round()
       // start slightly left of 50.5 so it will round to 50 instead of 51
+      //REVIEW add valueType: Vector2
       this.barrierLocationProperty = new Property( new Vector2( this.lattice.width / 2 - 1E-6, 0 ) );
 
       // @public {DerivedProperty.<number>} - the floor of the continuous barrier location (x coordinate only)
@@ -149,11 +150,13 @@ define( require => {
         barrierLocation => Math.round( barrierLocation.x )
       );
 
+      //REVIEW add value validation? range?
       // @public {NumberProperty} - width of the slit(s) opening in the units for this scene
       this.slitWidthProperty = new NumberProperty( config.initialSlitWidth, {
         units: this.positionUnits
       } );
 
+      //REVIEW add value validation?
       // @public distance between the center of the slits, in the units for this scene
       this.slitSeparationProperty = new NumberProperty( config.initialSlitSeparation, {
         units: this.positionUnits

js/common/model/WaterScene.js

@@ -30,7 +30,7 @@ define( require => {
       // gets propagated to the lattice via the water drops
       this.desiredFrequencyProperty = new NumberProperty( this.frequencyProperty.initialValue );
 
-      //REVIEW add value valdation?
+      //REVIEW add value validation?
       // @public - In the water Scene, the user specifies the desired source separation.  This is the position of
       // the faucets.  The sourceSeparationProperty indicates the sources of oscillation once the water has struck.
       this.desiredSourceSeparationProperty = new NumberProperty( this.sourceSeparationProperty.value );

js/diffraction/model/DiffractionModel.js

@@ -37,6 +37,7 @@ define( require => {
       this.numberOfLinesProperty = new NumberProperty( 10 );
       this.angleProperty = new NumberProperty( 0 );
 
+      //REVIEW: add validValues: ApertureType.VALUES
       // @public {Property.<ApertureType>} selected scene
       this.sceneProperty = new Property( ApertureType.CIRCLE );
     }

js/waves/model/WavesScreenModel.js

@@ -253,6 +253,7 @@ define( require => {
 
       const rotationRange = new Range( 0, 1 );
 
+      //REVIEW this is really odd. Especially since over in Perspective3DNode, const rotationAmount = Easing.CUBIC_IN_OUT.value( this.rotationAmountProperty.get() );
       // @public - amount the 3d view is rotated. 0 means top view, 1 means side view.
       this.rotationAmountProperty = new NumberProperty( 0, {
         range: rotationRange
@@ -268,10 +269,12 @@ define( require => {
 
       // @public {Property.<Vector2>} - model for the view coordinates of the base of the measuring tape
       // We use view coordinates so that nothing needs to be done when switching scenes and coordinate frames.
+      //REVIEW add valueType: Vector2
       this.measuringTapeBasePositionProperty = new Property( new Vector2( 200, 200 ) );
 
       // @public {Property.<Vector2>} - model for the view coordinates of the tip of the measuring tape
       // This position sets reasonable model defaults for each scene: 1.0cm, 50cm, 500nm
+      //REVIEW add valueType: Vector2
       this.measuringTapeTipPositionProperty = new Property( new Vector2( 250, 200 ) );
 
       // @public - Notifies listeners when the model reset is complete

js/waves/view/WavesScreenView.js

@@ -214,7 +214,7 @@ define( require => {
         0, this.latticeNode.globalBounds.top - intensityGraphPanel.getChartGlobalBounds().top
       );
 
-      //REVIEW Property's value parameter is not optional, use Property(null) here?
+      //REVIEW Property's value parameter is not optional, use Property(null) here? validate value?
       const measuringTapeProperty = new Property();
       model.sceneProperty.link( scene => {
         measuringTapeProperty.set( {