clojure-style-guide
clojure-style-guide copied to clipboard
Pre-conditions should not be used when IllegalArgumentException is appropriate
As Clojure is currently implemented, a failing pre- or post-condition throws an AssertionError, as opposed to an Exception. Errors are subclasses of Throwable but not of Exception: http://docs.oracle.com/javase/7/docs/api/java/lang/Error.html. Per the javadoc on Error: "An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch. Most such errors are abnormal conditions."
Things like ensuring an input is a positive number, ensuring certain keys are in a map, etc. are things that should typically be Exceptions.
http://programmers.stackexchange.com/questions/137158/is-it-better-to-use-assert-or-illegalargumentexception-for-required-method-param
What would you recommend writing, and how is it unique to Clojure?
As an example, If "ensuring input is a positive number" means "you promise to never give me a positive number", a precondition (and AssertionError) is correct.
The distinction is exactly as you've indicated -- whether or not the application should try to continue. Contracts are never meant to be broken, and failing to continue is the correct course of action when they are. I have used preconditions often to ensure certain keys are in a map to help during development time, then turned them off *assert* false
in production.
This isn't unique to Clojure at all, I simply think it is misleading to say "Prefer function pre and post conditions to checks inside a function's body" when in fact much of the time (doing validation), you actually want explicit checks that throw IAEs.
Perhaps I'm just bothered by the fact that the example given is not actually equivalent - the corollary for the precondition check is to either use an explicit "assert" statement or to have an "if" statement throwing an AssertionError, not an IAE. To compare the two, and say that using preconditions is "good" and throwing IAEs is "bad" is completely misleading, especially since (as you've agreed) the two are semantically totally different.
I might agree with @jakepic1.
The example should be:
;; bad
(defn foo [x]
(if (pos? x)
(bar x)
(throw (AssertionError. "x must be a positive number!")))
Or we have to decide if pre and post are preferred to the use of an assert:
;; bad
(defn foo [x]
(assert (pos? x)))
The assertions are meant to crash your app, and indicate a problem that is unrecoverable, and is why generally you can disable them in prod. IllegalArgumentException is generally looser, and is meant for things that can be recovered, so when input to the function is based on the runtime context, like from a user form, or a remote call, or a file, etc.
So it would be good to throw and catch IllegalArgumentException so that I can show my user a validation error, and he can correct his input.