On Mon, Oct 01, 2018 at 12:36:18PM -0400, David Mertz wrote:
There's some saying/joke about software testing along the lines of:
For an argument that should be in range 1-100: try 50; try 1; try 100 try 101; try 0; try -1, try 3.14 try 1+1j; try the string "fish", try a null pointer; etc.
Many of those oddball cases can easily be in a list of values to test in unit tests, but may be impossible or unlikely to make it to the function call in the normal/possible flow of the program.
Right. And for a *library*, you must handle out of range errors, because the library cannot trust the input passed to it by third parties. If a third party passes "fish" as an argument, that's not a bug in the library and the library cannot fix it, it should handle the error gracefully with a sensible error message. But for an *application*, the internal routines are completely under control of the one developer team. If an internal routine passes "fish", that's a bug that the dev team can and ought to fix. Once that bug is fixed, there's no need to check for "fish" in production code. Its a waste of time. (In principle at least -- it depends on how confident you or your QA team are that all the bugs are ironed out. Do you ship your apps with debugging turned off? Then you might ship with contract checking turned off, or at least dialled back to a lower level.) For an application, it doesn't matter if my function sets the computer on fire when passed the string "fish", if there is no way for the application to pass that string to the function. If it can't happen, it can't happen and there's no need to defend against it beyond a regression test. (Modulo comments about how confident you are about the software quality.) For a library, anyone can pass "fish" to anything, so you better handle it gracefully rather than setting the computer on fire. For application developers, the point of contracts is to make error conditions *impossible* rather than to just defend against them, so you can turn off the defensive error checking in production because they aren't needed.
I'm curious. When you write a function or method, do you include input checks? Here's an example from the Python stdlib (docstring removed for brevity):
# bisect.py def insort_right(a, x, lo=0, hi=None): if lo < 0: raise ValueError('lo must be non-negative') if hi is None: hi = len(a) while lo < hi: mid = (lo+hi)//2 if x < a[mid]: hi = mid else: lo = mid+1 a.insert(lo, x)
Do you consider that check for lo < 0 to be disruptive? How would you put that in a unit test?
I definitely put in checks like that. However, I might well write a test like:
assert lo >= 0, "lo must be non-negative"
That would allow disabling the check for production.
For a library function, I would consider that an abuse of assert, since: (1) It raises the wrong exception (AssertionError); (2) It can be disabled; (3) And it sends the wrong message, telling the reader that lo is an internal implementation detail rather than part of the function's external API.
However, I presume the stdlib does this for a reason; it presumably wants to allow callers to catch a specific exception. I haven't seen anything in the contract discussion that allows raising a particular exception when a contract is violated, only a general one for all contracts.
I think that the Eiffel philosophy is that all contract violations are assertion errors, but I'm not really confident in my understanding of Eiffel's exception mechanism. It doesn't have a generalised try...except statement like Python. If Python had contracts, I'd want to see: - by default, contract violations raise specific subclasses of AssertionError, e.g. RequireError, EnsureError; - but there ought to be a mechanism to allow specific errors to raise more specific exceptions. I haven't given any thought to that mechanism. It might simply be "the status quo" -- there's no reason why we *must* move defensive input checking into the (hypothetical) require block just because it is available. You can still write your test the old fashioned way in the body of the function and raise the exception of your choice.
The case of 'if hi is None' is different. It's remediation of a missing value where it's perfectly fine to impute an unspecified value. So that would be a poor contract.
Since the check for None is not error checking at all, I didn't talk about it or make it a precondition test. -- Steve