Allow string input, convert to Number via parseInt

[chriscalo]

trafficstars

Can save people a step if their input is a string representation of a number

[dcousens]

@chriscalo thanks for the PR!

This change incidentally adds a blind spot for developers though, in that, they can pass a string which doesn't parseInt to a finite number.

Granted, our Infinity and NaN handling isn't great as is, but, it was at least on the developer to verify. This doesn't improve that and in my opinion, increases the potential for that to be a problem.

[chriscalo]

The error of passing a string that parses to a non-finite number is still caught because the same validations run as before:

function ordinal (i) {
  if (typeof i === 'string') {
    i = parseInt(i);
  }
  if (typeof i !== 'number') throw new TypeError('Expected Number, got ' + (typeof i) + ' ' + i)
  
  if (!Number.isFinite(i)) return i
  return i + indicator(i)
}

The only difference is that, if the string is parseable to a Number, it does so and proceeds.

Is there another behavior that would be preferable that still accepts a number as a string?

[dcousens]

The error of passing a string that parses to a non-finite number is still caught because the same validations run as before:

Yes, but, now an error or (return i) is higher in likelihood - we are taking the responsibility of data validation from the developer.

Is there another behavior that would be preferable that still accepts a number as a string?

Probably, throw if parseInt fails - but this is an assumption of developer intent, and honestly I don't know if it is commonplace or not.

I think simple is better in this case.

[chriscalo]

Probably, throw if parseInt fails - but this is an assumption of developer intent, and honestly I don't know if it is commonplace or not.

It seems like parseInt() only returns an integer Number or NaN.

In the case of an integer Number, this is a value the rest of the function can work with.

In the case of NaN, that's handled by the call to Number.isFinite(). Are you saying it could throw there?

If so, happy to update and try to send over another PR or update this one.