Hi,
2012/11/27 Gilles Sadowski <gil...@harfang.homelinux.org> > 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. > > Actually, I would like the methods to be tested, so they cannot be private. That's the reason why I made them package private. If we remove the checks (which I really have mixed feelings about) I ropose to fully state in the javadoc that the preconditions are *not* checked, and that it's the responsibility of the client to check them. But in fact, may be it's too much risky work. It's probably better to leave the checks in place. > > Regards, > Gilles > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
