commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From S├ębastien Brisard <sebastien.bris...@m4x.org>
Subject Re: [math] Checking preconditions on package private functions
Date Tue, 27 Nov 2012 22:09:31 GMT
Hi,


2012/11/27 Gilles Sadowski <gilles@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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message