Hi Gilles, 2012/9/6 sebb <seb...@gmail.com>: > On 5 September 2012 22:46, Gilles Sadowski <gil...@harfang.homelinux.org> > wrote: >> On Wed, Sep 05, 2012 at 06:30:08PM -0000, l...@apache.org wrote: >>> Author: luc >>> Date: Wed Sep 5 18:30:08 2012 >>> New Revision: 1381284 >>> >>> URL: http://svn.apache.org/viewvc?rev=1381284&view=rev >>> Log: >>> Added throw declarations for package complex. >>> >>> Modified: >>> >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java >>> >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java >>> >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java >>> >>> Modified: >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java >>> URL: >>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284&r1=1381283&r2=1381284&view=diff >>> ============================================================================== >>> --- >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java >>> (original) >>> +++ >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java >>> Wed Sep 5 18:30:08 2012 >>> @@ -22,6 +22,7 @@ import java.util.ArrayList; >>> import java.util.List; >>> >>> import org.apache.commons.math3.FieldElement; >>> +import org.apache.commons.math3.exception.MathInternalError; >>> import org.apache.commons.math3.exception.NullArgumentException; >>> import org.apache.commons.math3.exception.NotPositiveException; >>> import org.apache.commons.math3.exception.util.LocalizedFormats; >>> @@ -566,12 +567,17 @@ public class Complex implements FieldEle >>> * @since 1.2 >>> */ >>> public Complex acos() { >>> - if (isNaN) { >>> - return NaN; >>> - } >>> + try { >>> + if (isNaN) { >>> + return NaN; >>> + } > > Regardless of whether it makes sense to catch NUA the above condition > should not be part of the try clause. > >>> >>> - return this.add(this.sqrt1z().multiply(I)).log() >>> - .multiply(I.negate()); >>> + return this.add(this.sqrt1z().multiply(I)).log() >>> + .multiply(I.negate()); >>> + } catch (NullArgumentException e) { >>> + // this should never happen as intermediat results are not null >>> + throw new MathInternalError(e); >>> + } >>> } >> >> Maybe I don't understand the purpose of catching "NullArgumentException" and >> rethrowing something else. >> >> Anyway, I was going to start a new thread about "NullArgumentException": my >> proposal is to never check for null and let the standard NPE be thrown in >> case of bad usage (null passed where a non-null is required). >> >> That would avoid such catch and rethrow for things that never happen. >> >> >> Best regards, >> Gilles >> >>> [...] >> I was about to start a new thread too, but refrained to do so for lack of knowledge on the history of this particular exception. Check for null is unevenly enforced througout the library, which --to me-- suggests we shouldn't do it at all and contend with NPE. There is one potential use, though. I think we should check for null when the NPE might occur in a different method.
This is what happens with new Incrementor(int, MaxCountExceededCallback) : cb is just copied, and fields of cb are invoked elsewhere: in that case, checking for null does make sense. Sébastien --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
