Sebb, Gilles and I would like to start a discussion about the use of
assert in CM code.
This started by comments in a JIRA ticket
(https://issues.apache.org/jira/browse/MATH-845), I rewrite below a
summary:
Gilles, commenting on my code:
"assert" statements should be replaced by precondition checks (raising
an appropriate exception if they fail).
myself:
In the case of private or package private methods, is it also required
to turn them into runtime checks ? Is it acceptable to simply comment
them if the performance impact is significant ?
Gilles:
AFAIK, there is no policy for the use of "assert" within the CM code.
In the case of a library like CM, robustness is important; I guess that
it is less safe to use "assert" since an application could silently fail
if assertions are not enabled.
In cases where the method is for internal use only, and we are sure
that the CM code calls it correctly, my preference would be to just
remove the unused statement, and replace it with an appropriate comment
stating why an explicit check is not necessary.
Sebb:
Interesting! I was about to ask the same question on the ML for another
issue (incomplete beta function).
I really would like to have this point discussed more widely, as I tend
to prefer assertions over simple comments (assertions could be turned on
(at least, locally) during testing).
Of course, I'm in favor of an additional comment stating that explicit
check is deemed unnecessary for private or package private methods. In
my case, some package private methods provide approximations of some
functions over a specific interval. I would then state in the javadoc
the interval, and also that it is the responsibility of the user to make
sure that this approximation is not called outside its domain.
Gilles:
IMO, neither "assert" statements nor simple comments are useful for
testing (or more precisely, debugging). What we'd need is logging.
"assert" aborts the program, but does not tell you how the wrong value
made it there.
I think that "assert" is very useful, but in applications (meaning:
don't bother with the rest of the computation, as I know that someting
went wrong).
By contrast, in CM, no "assert" should ever be triggered from internal
calls. Furthermore, no "assert" should ever be triggered from external
calls, as that would mean that unsafe (if assertions are disabled) code
can be called.
To be robust, CM raises a "MathInternalError" at places no code path
should ever lead to; but if one does, at least something predictable
happens.
One thing to check in the first place is whether a precondition check
actually slows down things in an "unbearable" way.
end of comments in MATH-845, let's discuss this in ML from now.
@Gilles: sometimes assertion are a form of comments on steroid -> they
will bark at you if you did not read them, and equally important: they
are unambiguous. When I first write a method, it usually does not work
right away. If it contains asserts, I usually get a clue of what's
wrong, and if I don't, I try to write additional ones before tracking
down the failure in a debug session. It is way faster, it forces me to
understand better what I am coding (when I just follow a algorithm
described in a paper, the typical situation for CM stuff) and the best
of all: it will help anyone who later modify the code.
-> in short, I see assertions as an invaluable tool to help developers,
not just something to check input parameters in private methods. For
this reason, I would support the following policy:
1. Use asserts as much as you like, but they are by no mean a
substitute to runtime checks (code that throws "MathInternalError") for
input parameters of public methods.
2. Asserts must be enabled in all unit tests
Sebastien
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
