Dan, my firm-but-not-rabid opinion on this has been formed from decades of tracking down portability bugs, irreproducible bugs, etc.
Return 't'. Try to get the contract tightened up to specify t.
But so long as the contract is loose, in testing it can be useful to try to inject defects by returning other values.
For any one instance of this, the odds of there being a bug based on the return value is low. But given how many functions return boolean values, the odds of none of the calls to them having a bug aren't so good. Making everything return 't' as a matter of standard practice has the potential to eliminate quite a number of bugs (in the sense of bad behavior, rather than in the sense of incorrect coding).
It's not just Lisp that has this problem. I've seen quite a number of C/C++ bugs where code erroneously depended on a return value of 1 or -1 or odd/even or positive/negative when it should have been zero/non-zero.
A similar area is when algorithms are sensitive to the order of traversal of hash tables. Java supplies LinkedHashTable and LinkedHashSet, which is one approach to removing this variability.
In both cases, the real issue isn't so much that there can be bugs. It is that the bugs that can result, have the potential to be very hard to reproduce, test for, and eliminate. An API contract that doesn't leave this sort of wiggle room makes for much better testability.
Of course, if everyone follows this rule, there's no need for: (when (fn1 arg2 arg2) t)
because fn1 would have returned t or nil -- unless you have an actual mismatch/conversion of contract. In that case, I argue that it is best to actually capture that intent in the code, as well as all the benefits above. But perhaps it should be written: (not (eq (fn1 arg2 arg2) null)) and reserve when for actual control-flow matters, rather than boolean operations.
Date: Fri, 14 Jan 2011 11:42:16 -0500 From: Daniel Weinreb dlw@itasoftware.com Subject: [pro] Style issue about predicates
If you have a function that is a predicate, in the sense that the function's contract says that its value should be interpreted as being either false or true, do you think it's better to code it so that it always returns "t" for the true case?
Since Common Lisp is quite clear that when a value is being considered in the context of being true/false, nil means false and everything else means true. So from a language point of view, even considering the "intent" of the definition and not just the spec, there is no need to return t.
Furthermore, the contract of the function should make it clear that the returned value is an a true/false context. This should be in the doc string, or at least in a comment, and the function name should end in "p" (or always "-p" but let's please not get into that in this email thread). So the caller should know.
All that said, it's possible that a programmer will fail to heed the contract, simply look at the code, and take advantage of the returned value in more than true/false context. If you want to prevent that, you can do something like:
(defun ... ... (when (fn1 arg2 arg2) t))
It seems that it might depend on the circumstance: how likely do you think it is that a programmer would commit such a mistake? The more potentially valuable the returned value is, the more likely. On the other hand, if it's so valuable, maybe you should actually make that part of the contract rather than making the function have the contract of a predicate.
Is this good, bad, don't care, depends on the circumstance?
-- Dan