Util.isInteger should throw an assertion error if a non-number is passed in.

[samreid]

Util.isInteger is currently defined like so:

    /**
     * Returns whether the input is a number that is an integer (no fractional part).
     * @public
     *
     * @param {number} n
     * @returns {boolean}
     */
    isInteger: function( n ) {
      return ( typeof n === 'number' ) && ( n % 1 === 0 );
    }

In the doc, the parameter type is described as {number} and indeed it would seem like passing string, objects or booleans to this function should flagged as a logic error. Hence, we should change typeof number to be an assert. I discussed this with @pixelzoom and he agreed we should move to the assert. I should also run fuzz test after this change to see if anything was relying on passing in non-numbers.

[samreid]

After making this change, I ran http://localhost/chipper/test-server/test-sims.html?loadOnly and all is well. Committing.

[samreid]

Committed above, @pixelzoom would you mind double checking the change?

[pixelzoom]

Looks good. But when an assertion fails, I like to have the offending value in the assertion message, like so:

assert && assert( typeof n === 'number', 'isInteger requires its argument to be a number: ' + n );

@samreid Look OK?

[samreid]

Yes, that is a good plan, closing.

[pixelzoom]

Reopening. I'd like to revisit this change. It's not ideal when using isInteger in contexts like this one, which I'm using extensively for type-checking in Natural Selection:

assert && assert( Utils.isInteger( value ) && value > 0, 'value must be a positive integer' );

If value is invalid not a number, then the assert in Utils.isInteger fails and I never see my more specific assertion message.

@samreid Any objection to reverting to the original implementation? And documenting it like this, to address the original complaint?

  /**
   * Determines whether a value is an integer.
   * @public
   *
   * @param {*} value
   * @returns {boolean}
   */
  isInteger: function( value ) {
    return ( typeof value === 'number' ) && ( value % 1 === 0 );
  },

[samreid]

That looks preferable, thanks! Also, please be apprised of https://github.com/phetsims/tasks/issues/1039 in which we may use Number.isInteger instead of % 1 === 0

[pixelzoom]

Changed in the above commit.

Thanks for the heads up on phetsims/tasks#1039. I'm going to ignore that for the purposes of this issues.

Closing.