On 9/19/12 12:12 AM, Sébastien Brisard wrote:
> Hi,
>
> 2012/9/18 Phil Steitz <phil.ste...@gmail.com>:
>> On 9/18/12 12:02 PM, Sébastien Brisard wrote:
>>> Hello,
>>>
>>> 2012/9/17 Gilles Sadowski <gil...@harfang.homelinux.org>:
>>>> On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote:
>>>>> OK, I give up. Lets do option 2. Just warn users in the User Guide
>>>>> somewhere that our APIs are in general not null-safe and unless the
>>>>> javadoc explicitly allows nulls, they can expect NPE when passing nulls.
>>>> Thanks, Phil; we are making progress, and I hope that you'll be convinced
>>>> of
>>>> that at some point.
>>>>
>>>> Another decision still needs to be made.
>>>>
>>>> I think that everybody now agrees that wherever an argument will be
>>>> directly
>>>> used (i.e. "dereferenced") inside the body of the method[1], it is safe to
>>>> not check for null (since the JVM will throw an NPE).
>>>>
>>>> But, whenever an argument is passed for _later_ use (e.g. stored by a
>>>> constructor or passed to another method), we also all expressed that it is
>>>> better to fail early if the object is supposed to be non-null. In those
>>>> cases, checks are not mandatory (since the JVM will throw NPE at some
>>>> point), but we must agree on how optional checks are to be implemented.
>>>> 1. The current state of affairs was to throw "NullArgumentException"; in
>>>> 4.0
>>>> this exception will become a subclass of NPE and we can continue to use
>>>> it in the same way as now (i.e. possibly providing additional
>>>> localizable
>>>> error messages).
>>>> 2. The alternative is to directly throw a standard NPE, but without any
>>>> message, since it wouldn't be localizable with the support of CM's
>>>> "context".
>>>>
>>>> So, which of those alternatives do people want to settle on?
>>>>
>>>>
>>>> Regards,
>>>> Gilles
>>>>
>>>> [1] As opposed to being passed on to another method.
>>>>
>>> I have another question: are we going to little by little remove all
>>> null checks that we feel are not absolutely necessary? I would not
>>> mind going through this cleaning phase while working on MATH-854.
>> I think we should wait to remove the existing null checks until
>> 4.0. This is a significant behavior change, especially for the ones
>> that have been in place and documented since 1.x. Removing the
>> checks and allowing NPEs to propagate will break apps that catch IAE.
>>
>> Phil
>>
> OK. I would like to say that I think this is going the right way: it
> is better to have no null checks than incomplete null checks which
> provide a false sense of security. For example, have a look to
> Array2DRowRealMatrix
>
>
> /**
> * Create a new RealMatrix using the input array as the underlying
> * data array.
> * If an array is built specially in order to be embedded in a
> * RealMatrix and not used directly, the {@code copyArray} may be
> * set to {@code false}. This will prevent the copying and improve
> * performance as no new array will be built and no data will be copied.
> *
> * @param d Data for new matrix.
> * @param copyArray if {@code true}, the input array will be copied,
> * otherwise it will be referenced.
> * @throws DimensionMismatchException if {@code d} is not rectangular
> * (not all rows have the same length) or empty.
> * @throws NullArgumentException if {@code d} is {@code null}.
> * @throws NoDataException if there are not at least one row and one
> column.
> * @see #Array2DRowRealMatrix(double[][])
> */
> public Array2DRowRealMatrix(final double[][] d, final boolean copyArray) {
> if (copyArray) {
> copyIn(d);
> } else {
> if (d == null) {
> throw new NullArgumentException();
> }
> final int nRows = d.length;
> if (nRows == 0) {
> throw new NoDataException(LocalizedFormats.AT_LEAST_ONE_ROW);
> }
> final int nCols = d[0].length;
> if (nCols == 0) {
> throw new
> NoDataException(LocalizedFormats.AT_LEAST_ONE_COLUMN);
> }
> for (int r = 1; r < nRows; r++) {
> if (d[r].length != nCols) {
> throw new DimensionMismatchException(d[r].length, nCols);
> }
> }
> data = d;
> }
> }
>
> I think if you want to be really helpful, a null check should also be
> carried out on each d[i]. I think I will leave it as is for now, but
> in 4.0, we should certainly remove this null check.
> Do you agree?
Yes, the API should be nullsafe or not. The half-way solution above
is no good and it makes the javadoc misleading. I would be OK
actually adding the check at the row level now (or catching the NPE
and throwing DME in its place) so what the javadoc says is true (d
null means NAE and d non-rectangular or empty means DME). On the
other hand, this is unlikely to ever happen in practice, so I would
leave as is for now.
Phil
> Sébastien
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
