taoensso / truss

Assertions micro-library for Clojure/Script
https://www.taoensso.com/truss
Eclipse Public License 1.0
302 stars 14 forks source link

README and example of :el should demonstrate non-set collection #4

Closed yonatane closed 7 years ago

yonatane commented 7 years ago

Since special predicate :el example (have [:el #{:a :b :c :d}] :b) is equivalent to using the set itself as a predicate as in (have #{:a :b :c :d} :b) and doesn't even yield a more interesting error message, the example should use a collection type other than a set, perhaps a vector: (have [:el [:a :b :c :d]] :b).

ptaoussanis commented 7 years ago

Hi there!

Thanks for asking about this. A friendly suggestion if I may?

Have been seeing a lot of open source issues lately of the form "You should x", or "The code should y". I wonder whether it wouldn't make for a friendlier, more welcoming environment if as a community we could instead try to encourage language of the form "Is there a reason for z?", etc.

For instance:

"In the example (have [:el #{:a :b :c :d}] :b), isn't the special :el predicate equivalent to using the set itself as a predicate? It doesn't seem to yield a more interesting error message. Maybe the example could use a collection type other than a set, ..." - etc.

Does that make sense / seem reasonable?

And to answer that question: they're not entirely equivalent since sets unfortunately don't work great as predicates in general if you'd like to try test for the inclusion of falsey values.

Specifically:

(have #{:a :b :c} :a) ; Works fine, equivalent to the form below
(have [:el #{:a :b :c}] :a)

(have #{:a :b nil} nil) ; Trips up, not the intended behaviour
(have [:el #{:a :b nil}] nil) ; Works as intended

This can be an easy thing to unexpectedly slip on, so I'd generally suggest sticking to the :el form as a habit to be sure.

the example should use a collection type other than a set, perhaps a vector: (have [:el [:a :b :c :d]] :b).

Actually while this does work (and might be convenient from time to time), I'd generally recommend trying to stick to using only sets with :el since anything else would involve a costly set construction on each call.

Thanks for pointing out that this was unclear, will try mod the README example.

Cheers! :-)

yonatane commented 7 years ago

Thank you Peter for pointing out falsey values, for your friendly suggestion and for this lib and the others I use :) Duly noted!

ptaoussanis commented 7 years ago

No problem, cheers! :-)