On Tue, Nov 27, 2012 at 07:22:26AM -0800, Phil Steitz wrote: > On 11/27/12 6:42 AM, Gilles Sadowski wrote: > > On Tue, Nov 27, 2012 at 06:24:26AM -0800, Ted Dunning wrote: > >> Actually, I would still recommend checks. You may know what the code does > >> now, but you can't trust either yourself or somebody else in the future. > >> Better to do the checks. > > I don't agree because in this case, the situation is akin to a bug. [And we > > don't introduce checks after each statement to ensure that the statement > > did what it should.] > > > > Those _private_ methods are there just to group a set of statements which > > are valid under the documented conditions. We should start to assume that > > the documentation could be wrong... [If it is, that's also a bug.] > > I agree with both of you :) > > Easy to forget, new people shooting selves in foot later, etc. - > good point. > > Violating clearly stated preconditions == bug - also good point. > > If you read carefully what Sebastien is proposing, it addresses > both. Key is to fully document in the javadoc what the > preconditions are and what exceptions will be thrown / what nonsense > will result if they are violated. Then unit tests validate > correctness of the javdoc. Given all of this, dropping the checks > just makes the contract a little trickier to specify. If this can > be done clearly, I am OK with dropping the checks. If not, I agree > with Ted it is better to just leave the checks in. The real risk of > pulling them out is when we later change the code that uses them and > forget or emasculate the client side checks.
> Depending on what > kinds of exceptions or meaninglessness happens when the > preconditions are violated, not encapsulating them in the methods > may also make documentation of the code that uses the methods > harder. That should not be the case here: the methods are an "implementation detail" (replacing a set of statements by another, more precise, under the stated conditions) that should not be Javadoc-documented; only code comments are useful to figure why those methods are called there. > So I would say look at each one and ask if a) its own > javadoc and b) javadoc of the methods that now use it can be > specified fully and meaningfully in its activation context without > the checks in the code. If "yes" go ahead and rip them out. If > "no" leave them in. All you say is of course correct in the context of "public" or "protected" or package-visible methods. My point is that _if_ the methods can be made "private", then we can assume that they are used properly there (which is the only place where they can be called). It's the same that we wouldn't check whether, say, we wrote a + b instead of a - b If we did write the wrong statement, it is indeed a bug, and we would just correct it, without writing additional checks to make sure that we do not reintroduce the same bug some time later; at most, we would write a code comment to remind future readers to be cautious about a possibly non-obvious statement. Regards, Gilles --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
