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

Use isValidValue to validate, instead of an observer. #50

Closed pixelzoom closed 3 years ago

pixelzoom commented 3 years ago

From code review #35:

  • [ ] Are Validator validation options (valueType, validValues, etc...) utilized?

In AbstractNLDBaseModel.js:

    this.pointValuesProperty = new Property( [ null, null ], { valueType: Array } );
    this.pointValuesProperty.link( pointValues => {
      assert && assert( pointValues.length === 2, 'There should always be 2 point values.' );
    } );

The pointValuesProperty observer can be replaced with something like:

    this.pointValuesProperty = new Property( [ null, null ], { 
      valueType: Array,
      isValidValue: value => value.length === 2
    } );

or a more complete/correct validation:

```js
    this.pointValuesProperty = new Property( [ null, null ], { 
      valueType: Array,
      isValidValue: array => Array.isArray( array ) && 
         _.every( array, element => typeof element === 'number' ) && 
        array.length === 2
    } );
SaurabhTotey commented 3 years ago

I implemented the second option, but I tweaked it so that it didn't fail on null values in the array. I'll close this issue, but feel free to reopen if something more should be done.