From dev-return-138447-apmail-commons-dev-archive=commons.apache.org@commons.apache.org Thu Sep 13 11:07:50 2012 Return-Path: X-Original-To: apmail-commons-dev-archive@www.apache.org Delivered-To: apmail-commons-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 60C639EE3 for ; Thu, 13 Sep 2012 11:07:50 +0000 (UTC) Received: (qmail 75011 invoked by uid 500); 13 Sep 2012 11:07:49 -0000 Delivered-To: apmail-commons-dev-archive@commons.apache.org Received: (qmail 74856 invoked by uid 500); 13 Sep 2012 11:07:48 -0000 Mailing-List: contact dev-help@commons.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Commons Developers List" Delivered-To: mailing list dev@commons.apache.org Received: (qmail 74812 invoked by uid 99); 13 Sep 2012 11:07:47 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 13 Sep 2012 11:07:47 +0000 X-ASF-Spam-Status: No, hits=2.0 required=5.0 tests=RCVD_IN_BL_SPAMCOP_NET,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of SRS0=y7qJ=HM=m4x.org=sebastien.brisard@bounces.m4x.org designates 91.121.62.107 as permitted sender) Received: from [91.121.62.107] (HELO mx3.polytechnique.org) (91.121.62.107) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 13 Sep 2012 11:07:40 +0000 Received: from mx3.polytechnique.org (mx3 [91.121.62.107]) by ozgurluk.polytechnique.org (Postfix) with ESMTP id BB7A11E0132 for ; Thu, 13 Sep 2012 13:07:16 +0200 (CEST) Received: from mx1.polytechnique.org (mx1 [129.104.30.34]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx1.polytechnique.org", Issuer "Polytechnique.org" (verified OK)) by mx3.polytechnique.org (Postfix) with ESMTPS id AE50D1E012D for ; Thu, 13 Sep 2012 13:07:16 +0200 (CEST) Received: from mail-ie0-f171.google.com (mail-ie0-f171.google.com [209.85.223.171]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by ssl.polytechnique.org (Postfix) with ESMTPSA id 028821407E950 for ; Thu, 13 Sep 2012 13:07:15 +0200 (CEST) Received: by ieje14 with SMTP id e14so5984393iej.30 for ; Thu, 13 Sep 2012 04:07:14 -0700 (PDT) MIME-Version: 1.0 Received: by 10.42.170.73 with SMTP id e9mr1707615icz.11.1347534434659; Thu, 13 Sep 2012 04:07:14 -0700 (PDT) Received: by 10.64.141.167 with HTTP; Thu, 13 Sep 2012 04:07:14 -0700 (PDT) In-Reply-To: <20120913104820.GK20488@dusk.harfang.homelinux.org> References: <20120913095853.GJ20488@dusk.harfang.homelinux.org> <20120913104820.GK20488@dusk.harfang.homelinux.org> Date: Thu, 13 Sep 2012 13:07:14 +0200 Message-ID: Subject: Re: [math] Documenting exceptions in interfaces (MATH-854) From: =?ISO-8859-1?Q?S=E9bastien_Brisard?= To: Commons Developers List Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-AV-Checked: ClamAV using ClamSMTP at svoboda.polytechnique.org (Thu Sep 13 13:07:16 2012 +0200 (CEST)) X-Org-Mail: sebastien.brisard.1997@polytechnique.org X-Virus-Checked: Checked by ClamAV on apache.org X-Old-Spam-Flag: No, tests=bogofilter, spamicity=0.000165, queueID=68EAC1407E954 Hi, 2012/9/13 Gilles Sadowski : > On Thu, Sep 13, 2012 at 12:04:52PM +0200, S=E9bastien Brisard wrote: >> Hi, >> >> 2012/9/13 Gilles Sadowski : >> > Hello. >> > >> > I'm also feeling tired of those issues. I must point out that this see= ms so >> > complicated _because_ we depart from best practices (as finely describ= ed in >> > e.g. "Effective Java"). >> > Whatever seems a help (and probably is sometimes) in one direction lea= ds to >> > inconsistencies in another, like in this case, where advertizing runti= me >> > 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 no= t >> > interchangeable with checked exceptions: this is _not_ because checked >> > exceptions are provided by the Java compiler as a help to the develope= rs but >> > because they describe _fundamentally_ different failures. Not acknowle= dging >> > that will cause headaches.] >> > >> > As for CM and the "trick", what's important for Luc (as a _user_) is t= he >> > "throws" _clause_ IIUC, not the Javadoc "@throws" _tag_: by turning th= e >> > 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 excepti= ons > in interfaces, I had been meaning that the _documentation_ should not con= tain > false statements (like when you wrote, in the Javadoc, that > "FieldElement.divide" throws "MathArithmeticException" when the divisor i= s > 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 c= omes >> > with the "help" which we decided to provide. >> > >> > One way out of this mess might be to indeed fill _all_ the "@throws" t= ags >> > (OK for the "trick" and OK for CheckStyle) but to add something like >> > "implementation-dependent"; e.g. for "divide" in "FieldElement": >> > >> > /** >> > * Computes this ÷ a. >> > * >> > * @param a Divisor. >> > * @return a new element representing this ÷ a >> > * @throws NullArgumentException if {@code a} is {@code null}. >> > * Implementation-dependent. >> > * @throws MathArithmeticException if {@code a} is zero. >> > * Implementation-dependent. >> > */ >> > T divide(T a) throws NullArgumentException, MathArithmeticExceptio= n; >> > >> > 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 "tri= ck" > to work (at least if the code is supposed to be compilable). That the thr= ows > 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 trigg= ers > CheckStyle warnings, so I propose to revert to putting them there, but wi= th > 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, invitin= g > 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 o= ne > step further (too far?)...]. > Yes, that might be a bit too far... Sorry for having misunderstood. Best regards, S=E9bastien > > Best, > Gilles > > >> >> Thanks for taking the time to answer! >> Best regards, >> S=E9bastien >> > >> > 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, t= his 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 sam= e size as >> >> * {@code this}. >> >> */ >> >> >> >> instead of simply writing /** {@inheritDoc} */. >> >> >> >> 2. The resulting javadoc of implementations is not satisfactory. For >> >> example, the javadoc of FieldVector.add(FieldVecto 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> >> >> 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.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.divide(). >> >> 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 th= e >> >> 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 shoul= d >> >> 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 realiz= e >> >> 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 reall= y >> >> would rather we had more time to concentrate on real (meaning, >> >> numerical) issues. >> >> >> >> S=E9bastien >> >> >> >> [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