Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: neutral (nike.apache.org: local policy)
Message-ID: <4D7381C2.1040302@free.fr>
Date: Sun, 06 Mar 2011 13:44:50 +0100
From: Luc Maisonobe <Luc.Maisonobe@free.fr>
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US;
 rv:1.9.2.14) Gecko/20110223 Thunderbird/3.1.8
MIME-Version: 1.0
To: Commons Developers List <dev@commons.apache.org>
Subject: Re: [Math - 403] Never propagate a "NullPointerException" resulting
 from bad usage of the API
References: <20110302113717.GN22814@dusk.harfang.homelinux.org>
 <4D6E46FB.80608@free.fr> <20110302230832.GJ22170@dusk.harfang.homelinux.org>
 <4D6F7512.5070308@free.fr>
 <20110303153446.GQ22814@dusk.harfang.homelinux.org>
 <4D6FBC10.1030703@free.fr>
 <20110303225116.GK22170@dusk.harfang.homelinux.org>
 <4D70C67A.9070808@free.fr>
 <20110304125544.GL22170@dusk.harfang.homelinux.org>
 <4D71097C.8070405@free.fr>
 <20110304235934.GO22170@dusk.harfang.homelinux.org>
In-Reply-To: <20110304235934.GO22170@dusk.harfang.homelinux.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8bit

Le 05/03/2011 00:59, Gilles Sadowski a �crit :
> Hello.

Hi Gilles,

> 
>> [...]
>>> So, what is the lesson from having this case? Is the whole method a duplicate
>>> that should be replace by a call to something in "linear"? Or is it a proof
>>> that some exceptions might be appropriately used outside of "linear"?
>>
>> I think the method should be moved to a new
>> TruncatedCholeskyDecomposition or RectangularCholeskyDecomposition. I
>> don't think we can add it has a new implementation or it should be added
>> to the existing CholeskyDecomposition because the decomposition matrix
>> shape is not triangular.
> 
> If you are going to create new classes in "linear" so that the current CM
> code only throws "...MatrixException"s from inside that package, I'm not
> going argue anymore about moving those exceptions to "linear". But, if this
> situation arises again, then we will create a incompatibility by moving the
> exception from "linear" to "exception"... That's why I think that it's safer
> (from a stability point-of-view) to have them all in "exception".

I don't think this will happen very often.
So let's create the new classes (I'll open a Jira issue for that) and
move the linear algebra related exceptions back to the linear package.

> 
>>>>> [...]
>>>>
>>>> For example to have a single point where to put breakpoints for
>>>> debugging. I used that a lot as such exceptions mainly occur during
>>>> development phase.
>>>
>>> Thus, IIUC, this is a developer feature.
>>> I understand the convenience, but I wouldn't trade code consistency for it.
>>> I'd rather keep "NullArgumentException" (or a "MathNullPointerException" if
>>> we want it to extend the standard NPE) so that you can put your breakpoint
>>> in the constructor of that exception. Thus you have the convenience without
>>> the code asymmetry.
>>
>> OK for MathNullPointerException.
> 
> With a "checkNotNull" function, it should not be necessary.
> But I didn't quite understood your reasoning concerning such a method and
> whether it's OK to implement or not (cf. below).
> 
>>>
>>>>>
>>>>> Not only is this inconvenient, it is also dangerous: I was bitten more than
>>>>> once writing this:
>>>>> ---
>>>>>   MathRuntimeException.createNullPointerException();
>>>>> ---
>>>>> which the compiler happily accepted.
>>>>
>>>> Of course, as it would have accepted:
>>>>
>>>>   new NullPointerException();
>>>>
>>>> Unfortunately, we cannot put the throw inside the method. Trying to do
>>>> this prevent the optimizer to wor properly as it does not know in the
>>>> caller that the method would never return, and it brings lots of
>>>> warnings from code checking tools for the same reason.
>>>
>>> The method alternative looses much of its appeal because it doesn't perform
>>> the complete action ("throw" included), as I had initially assumed when
>>> wrongly leaving out the "throw" statement. I wouldn't have written
>>>   new NullPointerException();
>>> as, to me, it would have _obviously_ looked wrong.
>>>
>>> I don't understand the problem with the "optimizer". There are several
>>> methods in CM that seem to perform similarly (i.e. they check something and
>>> throw an exception). An example is "checkOrder" in "MathUtils".
>>> In fact we could add a "checkNotNull" in "MathUtils":
>>> ---
>>> public checkNotNull(Object obj) {
>>>   if (obj == null) {
>>>     throw new NullPointerException();
>>>   }
>>> }
>>> ---
>>> Wouldn't that satisfy all needs (ability to set a breakpoint and no factory
>>> methods)?
>>
>> No the check may succeed or not, so sometimes these methods do return.
> 
> I'd say that they return most of the time, and that's fortunate... :-)

Sorry, I misunderstood what you were writing. I was still talking about
the changing the createXxxException (which only creates) into
throwXxxException (which creates and throws), not about
checkXxxCondition (which checks, creates and throws).

> 
>> What I had in mind was something like:
>>
>>   public static void throwNPE() {
>>      throw new MySpecialNPE extends NullPointerException() {
>>         // my special code here
>>      };
>>   }
>>
>> Such a method *never* returns but on the caller side, the compiler, and
>> the checking tools don't know it. Typically when you have code like:
>>
>>   int var;
>>   if (obj != null) {
>>     var = obj.getVar();
>>   } else {
>>     throwNPE();
>>   }
>>
>>   // use var
>>
>> Then tools will complain that var may not be initialized, which is
>> false. However it is only because throwNPE never returns that it is false.
> 
> Personally, I don't like this kind of syntax. The good programming rule is
> to assign at declaration. [Unfortunately, one is sometimes forced to write
> in this way and most often than not because of checked exceptions!]
> 
>> Another way to put it that would probably work is:
>>
>>   public static void checkNonNull(Object o) {
>>     if (o == null) {
>>        throw new MySpecialNPE extends NullPointerException() {
>>           // my special code here
>>        };
>>     }
>>   }
> 
> How is this different from my version above?

It's the same, I simply get confused. So go for it, but including the
specialization and localization part that was in the former
createxxxException. So basically we add an if (obj == null) wrapper
aroud the 2.1 code.

best regards,
Luc

> 
>> Then the caller would do:
>>
>>  checkNonNull(obj);
>>  int var = obj.getVar();
> 
> That's exactly the intended usage!

> 
> 
> Gilles
> 
> ---------------------------------------------------------------------
> 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