groupon / assertive

Assertive is a terse yet expressive assertion library
BSD 3-Clause "New" or "Revised" License
21 stars 11 forks source link

Implemented hasType and notHasType #6

Closed johan closed 10 years ago

johan commented 10 years ago

Based on @dbushong's implementation in #2, integrating my feedback for that branch, and mergeable with current master. Error messages for the negative assertions also fixed, and I added Date, NaN, null and undefined "types" too (latter three being rather more special case constants that may happen) besides those in David's original branch.

johan commented 10 years ago

Devil-in-the-details notes (which are technically in the docs, but without a big red arrow pointing at them, or a block of text five mile long for the one person that might be interested):

Worth noting: this considers NaN as not a Number, which, while wrong in the IEEE sense for people that interpret reality on the ECMAscript bytecode level by the language, is probably the 99% right thing to do given the use case for an assertion library: asserting normal conditions an app handles, hardening against extraordinary circumstances, and the like.

(underscore, with its different use case, duck types NaN as a Number, though it doesn't math as a number, to all sorts of concerned people's horror, since two years ago. Not a bug, for them.)

On a similar note, my Date type test, unlike underscore, considers the Date class' provisions for holding a Date[Invalid Date] (that you can't math, or get sane data out of), as not a Date.

This might be worth mentioning this in docs, as it's technically bending the simplicity of the README spec, albeit for good intentions. I'd not go as far as introducing new Date('Invalid Date') as a full-blown type per the mechanics of this code; it's such a rare edge case people that hit it should craft their own assertions to cater to their one-in-ten-thousand needs.

EndangeredMassa commented 10 years ago

This looks good to me, but isType makes more sense to me than hasType.

johan commented 10 years ago

I think @dbushong's naming is intentional and avoids a semantic ambiguity issue "isType" has (are we asserting being the type, or being an instance of the type).

An object's type in js is in the strictest sense its constructor function, so more proper names of the function (to dodge that) would be "isOfType" or "isInstanceOfType". Compared to those, "hasType" is both unambiguous and more readable.

This may not be how David was reasoning, but it's at least why I like the name. (I was admittedly more lax about the naming of the unexposed helper functions I ended up needing.)

EndangeredMassa commented 10 years ago

Sounds good to me.

EndangeredMassa commented 10 years ago

:thumbsup:

johan commented 10 years ago

Published in 1.3.2.