commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: [Math] MathUtils.checkOrder
Date Thu, 22 Sep 2011 01:27:12 GMT
On 9/21/11 6:11 PM, Greg Sterijevski wrote:
> One more question, there is a boolean argument called 'abort', what sense
> does it make to keep checking an array given you have found one observation
> which violates monotonicity? I think abort is redundant and could be
> eliminated. Thoughts?

Looks like what is there now actually combines "isMonotone" and
"checkMonotone."  It returns a boolean and the abort parameter tells
it whether or not to throw instead of returning false.  I would see
a better separation of concerns to separate the two methods and let
the caller decide which one to use and whether or not (and what) to
throw when using isMonotone.

You are right that the impl looks like it could be improved to stop
checking when it has found a violation.

Phil
>
> On Wed, Sep 21, 2011 at 7:41 PM, Phil Steitz <phil.steitz@gmail.com> wrote:
>
>> On 9/21/11 4:33 PM, Greg Sterijevski wrote:
>>> Gilles,
>>>
>>> I do not understand why a non-monotone collection should throw a
>>> IllegalArgumentException...? There is nothing wrong with the argument, it
>>> just is not in corrected order. Wouldn't it be better to return a false?
>> I think as you guys are pretty much agreeing, we are talking about
>> two different methods here.  The "check*" methods are really there
>> to help with parameter checking, so it makes sense for them to throw
>> when what they are "checking" fails.  What you want should probably
>> be called "isMonotone."  That would also be useful and could be
>> called by the check method.
>>
>> As a side note, I notice now that "NonMonotonousSequenceException"
>> is misnamed.  It should be "NonMonotoneSequenceException."  I think
>> it would be good to fix that for 3.0.
>>
>> Phil
>>
>>> We have:
>>>
>>>             if (!ok && abort) {
>>>                 throw new NonMonotonousSequenceException(val[i],
>> previous,
>>> i, dir, strict);
>>>             }
>>>
>>> Why throw this? Why not return false and let the code calling this method
>>> decide if it wants to throw an exception?
>>>
>>> On Wed, Sep 21, 2011 at 5:56 PM, Gilles Sadowski <
>>> gilles@harfang.homelinux.org> wrote:
>>>
>>>> On Wed, Sep 21, 2011 at 05:17:59PM -0500, Greg Sterijevski wrote:
>>>>> Meant to say add, not replace. My apologies. -Greg
>>>> I like this better! ;-)
>>>> [But, still, please check the intended meaning of the first argument of
>>>> (sub-classes of) "MathIllegalArgumentException".]
>>>>
>>>> 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
>>
>>


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


Mime
View raw message