QueryStringMachine 'isValidValue' should return true or false

[pixelzoom]

Noted while working on https://github.com/phetsims/query-string-machine/issues/15.

There's currently an assumption that 'isValidValue' does its own error handling. E.g. in test-query-string-machine.js:

    name: {
      type: 'string',
      defaultValue: 'Larry',
      isValidValue: function( str ) {
        testAssert( str.indexOf( 'Z' ) !== 0, 'Name cannot start with Z: ' + str );
      }
    },

It would be preferable to return a boolean, like this:

      isValidValue: function( str ) {
        return str.indexOf( 'Z' ) !== 0; // cannot start with 'Z'
      }

... then provide uniform handling for this in QueryStringMachine.

This line in QueryStringMachine.validateValue would need to change:

235       schema.isValidValue && schema.isValidValue( value );

[pixelzoom]

Also note that we changed the name of isValidValue to be consistent with the similar option in Property, where it returns true or false.

As an example... In BLLQueryParameters, this is how isValidValue should be used. BLLQueryParameters should not be responsible for how an invalid value is handled, only for checking whether the value is valid.

    // {number} snap interval for the cuvette in centimeters, or 0 for no snap
    cuvetteSnapInterval: {
      type: 'number',
      defaultValue: BLLConstants.DEFAULT_CUVETTE_SNAP_INTERVAL,
      isValidValue: function( value ) {
        return value >= 0;
      }
    }

[samreid]

I agree, we should change QueryStringMachine.isValidValue to return boolean, not throw assertion errors.

[pixelzoom]

To address this, change line 235 of QueryStringMachine from:

schema.isValidValue && schema.isValidValue( value );

to:

schema.isValidValue && queryStringMachineAssert( schema.isValidValue( value ), key, prefix + 'value not allowed: ' + value );

[pixelzoom]

Changed as described in https://github.com/phetsims/query-string-machine/issues/16#issuecomment-256719053. @samreid please verify. You can use test-query-string-machine.html, BLL, or Plinko Probability.

[samreid]

Code changes look good, testing with http://localhost/query-string-machine/test-query-string-machine.html?name=Zarry looks good too, thanks! Closing.