commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: [math] Documenting exceptions in interfaces (MATH-854)
Date Thu, 13 Sep 2012 10:48:20 GMT
On Thu, Sep 13, 2012 at 12:04:52PM +0200, Sébastien Brisard wrote:
> Hi,
> 
> 2012/9/13 Gilles Sadowski <gilles@harfang.homelinux.org>:
> > Hello.
> >
> > I'm also feeling tired of those issues. I must point out that this seems so
> > complicated _because_ we depart from best practices (as finely described in
> > e.g. "Effective Java").
> > Whatever seems a help (and probably is sometimes) in one direction leads to
> > inconsistencies in another, like in this case, where advertizing runtime
> > exceptions, as a tool to improve the documentation, leads to the
> > documentation becoming wrong in some places!
> > [I think that I mentioned at some point that runtime exceptions are not
> > interchangeable with checked exceptions: this is _not_ because checked
> > exceptions are provided by the Java compiler as a help to the developers but
> > because they describe _fundamentally_ different failures. Not acknowledging
> > that will cause headaches.]
> >
> > As for CM and the "trick", what's important for Luc (as a _user_) is the
> > "throws" _clause_ IIUC, not the Javadoc "@throws" _tag_: by turning the
> > exception base class into a checked exception, he is informed of which
> > exceptions can be thrown by the code he calls.
> >
> 
> No, he is not, unless the exceptions are also specified in the
> signature of interface methods.
> That's my whole point!

I know!
There was a misunderstanding: When we discussed about advertizing exceptions
in interfaces, I had been meaning that the _documentation_ should not contain
false statements (like when you wrote, in the Javadoc, that
"FieldElement.divide" throws "MathArithmeticException" when the divisor is
zero, although "Complex.divide" actually does not).

> 
> >
> > That "help" should not entail that CM's doc should become riddled with false
> > statements[1], even if CheckStyle complaints that a "throws" clause is not
> > matched with a "@throws" tag![2]
> > I agree that this is not a nice situation but that's a drawback that comes
> > with the "help" which we decided to provide.
> >
> > One way out of this mess might be to indeed fill _all_ the "@throws" tags
> > (OK for the "trick" and OK for CheckStyle) but to add something like
> > "implementation-dependent"; e.g. for "divide" in "FieldElement":
> >
> >     /**
> >      * Computes this &divide; a.
> >      *
> >      * @param a Divisor.
> >      * @return a new element representing this &divide; a
> >      * @throws NullArgumentException if {@code a} is {@code null}.
> >      * <em>Implementation-dependent</em>.
> >      * @throws MathArithmeticException if {@code a} is zero.
> >      * <em>Implementation-dependent</em>.
> >      */
> >     T divide(T a) throws NullArgumentException, MathArithmeticException;
> >
> > This might be a little confusing for Javadoc readers (and should be
> > explained in the user guide) but at least it is correct (OK for the
> > documentation!).
> >
> > What do you think?
> >
> I think that's OK, but you really are doing what you said shouldn't be
> done: specify exceptions in interfaces. That's what I think ought to
> be done, otherwise, the whole thing is pointless. Or did I
> misunderstand?

Indeed, the "throws" clauses *must* be filled all the way up for the "trick"
to work (at least if the code is supposed to be compilable). That the throws
clauses must exactly match all the way, up and down, is the contract
enforced by checked exceptions.

What I proposed in my previous mail was to suggest what _could_ be done
by implementations. Not putting these suggestions in "@throws" tags triggers
CheckStyle warnings, so I propose to revert to putting them there, but with
a strongly spelled-out warning that it is not a contract!

[What can I say? Personally, I never found this nice, nor useful. IMO, it's
great that Java has come with an integrated documentation system, inviting
developers to write the documentation while writing the code; but the
documentation is still written by humans, for humans. It is normal that
gaps/typos/mistakes can happen in the documentation; the _policy_ is to
improve the documentation wherever possible, but we _cannot_ expect this
policy to be implemented automatically. We tried to push the automation one
step further (too far?)...].


Best,
Gilles


> 
> Thanks for taking the time to answer!
> Best regards,
> Sébastien
> >
> > Regards,
> > Gilles
> >
> > [1] Like has appeared with "over-documenting" exceptions (cf. "Complex" and
> >     "Decimal64").
> > [2] Maybe there is a way to deactivate the warning in places where we are sure
> >     that should not complain about a specific "@throws" tag. [Aggre, this is
> >     becoming very heavy...]
> >
> >> in previous discussions, it was decided that Interfaces (and, I
> >> suppose abstract methods) should *not* have a throws clause.
> >> So, yesterday, I started modifying the javadoc of FieldVector. Each
> >> "throws" clause was simply replaced by the following statement
> >> "Implementations should throw [...] if [...]". Please have a look to
> >> FieldVector and ArrayFieldVector for clarity.
> >> This has several drawbacks
> >>
> >> 1. The javadoc of implementations must grow, since the implementer
> >> must write something like
> >>
> >>     /**
> >>      * {@inheritDoc}
> >>      *
> >>      * @throws DimensionMismatchException if {@code v} is not the same size
as
> >>      * {@code this}.
> >>      */
> >>
> >> instead of simply writing /** {@inheritDoc} */.
> >>
> >> 2. The resulting javadoc of implementations is not satisfactory. For
> >> example, the javadoc of FieldVector<T>.add(FieldVecto<T> v) now
reads
> >>
> >> // Begin Javadoc
> >> Compute the sum of this and v. Implementations should throw
> >> DimensionMismatchException if v is not the same size as this.
> >>
> >> Specified by:
> >>     add in interface FieldVector<T extends FieldElement<T>>
> >> Parameters:
> >>     v - vector to be added
> >> Returns:
> >>     this + v
> >> Throws:
> >>     DimensionMismatchException - if v is not the same size as this.
> >> // End javadoc
> >>
> >> The "should throw" statement should really not be here, but it is too
> >> much of a hassle to rewrite the whole javadoc comment for each
> >> implementation.
> >>
> >> 3. Using Luc's trick brings a whole lot of error messages
> >>
> >> // Begin error message
> >> Exception MathXxxException is not compatible with throws clause in [...]
> >> // End error message
> >>
> >> this is not really a problem, but it makes the whole process of
> >> populating the throws clauses a bit difficult.
> >>
> >> 4. More importantly, there is *no way* to ensure that we actually
> >> document all exceptions. Indeed, if we take for example
> >> FieldVector<T>.mapDivide(T d)
> >>
> >> The only reason we know we *have* to add MathArithmeticException to
> >> the throws clause is because FieldElement (which is an interface)
> >> *specifies* this exception in the throws clause of
> >> FieldElement<T>.divide(<T>).
> >> If this throws clause is removed from interfaces, then LUC'S TRICK
> >> becomes useless. [1]
> >>
> >> For all these reasons, I would advocate *specifying* in interfaces
> >> exceptions which we know must occur. For example,
> >> DimensionMismatchException will be in the signature of *all*
> >> implementations of FieldVector.add(FieldVector). Why not add it to the
> >> throws clause? The answer is likely to be "because it is bad
> >> practice", but I think advertising unchecked exceptions is already a
> >> bad practice. So I think if we go for a bad practice anyway, we should
> >> do it *only if it makes our lives easier*. I don't think the current
> >> state does.
> >>
> >> On a more personal side, I'd like to say that I'm getting tired of
> >> this issue. I have been working for days on the linear package, but
> >> I'm making no progress, because each time I commit a change, I realize
> >> this was not the proper thing to do because of new exchanges on the
> >> ML. So I keep going back and forth. This is really sucking all of my
> >> C-M time, while I'd like to be working on other issues (eg special
> >> functions Gamma and Beta, visitors for FieldVectors, ...). That would
> >> be perfectly fine if I could see the benefit of MATH-854. While this
> >> seemed a good idea when we started discussing it, I'm not sure
> >> anymore, now that we have really tried to implement MATH-854.
> >>
> >> I'm sure that I'm not the only one among the regular developers to
> >> spend so much time on this issue. Our powers are limited, and I really
> >> would rather we had more time to concentrate on real (meaning,
> >> numerical) issues.
> >>
> >> Sébastien
> >>
> >> [1] MathArithmeticException in FieldElement.divide(FieldElement) is
> >> probably not the best example, as Gilles noted inconsistencies
> >> (Decimal64 and Complex do not throw an exception, but return NaN
> >> instead).
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
> >>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message