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] Documenting exceptions in interfaces (MATH-854)
Date Thu, 13 Sep 2012 11:07:14 GMT
Hi,

2012/9/13 Gilles Sadowski <gilles@harfang.homelinux.org>:
> 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).
>
Right, this means I must revert (again!) my changes... We will get
there, eventually.

>>
>> >
>> > 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?)...].
>
Yes, that might be a bit too far...
Sorry for having misunderstood.
Best regards,
Sébastien
>
> 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
>


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


Mime
View raw message