You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Greg Sterijevski <gs...@gmail.com> on 2011/09/22 00:05:47 UTC
[Math] MathUtils.checkOrder
Hello All,
In MathUtils there exists the method:
public static boolean checkOrder(double[] val, OrderDirection dir,
boolean strict, boolean abort) {
...code omitted...
}
I would like to replace it with the method:
public static boolean checkOrder(Comparable[] val, OrderDirection dir,
boolean strict, boolean abort ){
Comparable previous = val[0];
boolean ok = true;
int max = val.length;
int comp;
for (int i = 1; i < max; i++) {
comp = val[i].compareTo(previous);
switch (dir) {
case INCREASING:
if (strict) {
if (0 <= comp) {
ok = false;
}
} else {
if ( comp < 0) {
ok = false;
}
}
break;
case DECREASING:
if (strict) {
if (comp >= 0) {
ok = false;
}
} else {
if (comp > 0) {
ok = false;
}
}
break;
default:
// Should never happen.
throw new IllegalArgumentException();
}
if (!ok && abort) {
throw new NonMonotonousSequenceException(val[i], previous,
i, dir, strict);
}
previous = val[i];
}
return ok;
}
Would this be acceptable to all?
I would also need to change NonMonotonousSequenceException. It would take
comparables in the constructor and look like:
public NonMonotonousSequenceException(Comparable wrong,
Comparable previous,
int index,
MathUtils.OrderDirection
direction,
boolean strict) {
super(direction == MathUtils.OrderDirection.INCREASING ?
(strict ?
LocalizedFormats.NOT_STRICTLY_INCREASING_SEQUENCE :
LocalizedFormats.NOT_INCREASING_SEQUENCE) :
(strict ?
LocalizedFormats.NOT_STRICTLY_DECREASING_SEQUENCE :
LocalizedFormats.NOT_DECREASING_SEQUENCE),
(Number) previous.compareTo(wrong),
(Number)0, index, index - 1);
this.direction = direction;
this.strict = strict;
this.index = index;
this.previous = null;
this.previousComp = previous;
}
Thoughts?
Thanks!
-Greg
Re: [Math] MathUtils.checkOrder
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.
>
> 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?
>
> 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?
Did you have a look to the various overloads of "checkOrder"?
In the above, if "abort" is "false", no exceptions will be thrown!
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] MathUtils.checkOrder
Posted by Greg Sterijevski <gs...@gmail.com>.
If there are no objections, I will move the body of the current
checkOrder(double[] arg, ...) into a isMonotone method. I will also create a
parallel set of checkOrder, isMonotone functions for Comparable[] arrays.
-Greg
On Wed, Sep 21, 2011 at 7:41 PM, Phil Steitz <ph...@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
>
>
Re: [Math] MathUtils.checkOrder
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Wed, Sep 21, 2011 at 06:27:12PM -0700, Phil Steitz wrote:
> 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.
Yes, sorry for that. That was indeed the intention.
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] MathUtils.checkOrder
Posted by Phil Steitz <ph...@gmail.com>.
On 9/22/11 3:02 AM, Gilles Sadowski wrote:
>>>>> 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.
> How about compromising on "NonMonotonicSequenceException"?
Fine by me.
Phil
>
>
> 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
Re: [Math] MathUtils.checkOrder
Posted by Greg Sterijevski <gs...@gmail.com>.
Either is fine by me... ;) In case my opinion was solicited.
On Thu, Sep 22, 2011 at 5:02 AM, Gilles Sadowski <
gilles@harfang.homelinux.org> wrote:
> > > >> 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.
>
> How about compromising on "NonMonotonicSequenceException"?
>
>
> Gilles
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [Math] MathUtils.checkOrder
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
> > >> 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.
How about compromising on "NonMonotonicSequenceException"?
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] MathUtils.checkOrder
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Thu, Sep 22, 2011 at 08:58:19AM -0500, Greg Sterijevski wrote:
> I agree with your assessment that having almost identical methods is a pain.
> However, without doing this I need to return a very complicated set of
> information from isMonotone to be able to construct the exception.
Yes, this would be ugly indeed.
> As for catching the exception, I was under the impression that CM code never
> catches exceptions, you propagate them upwards on the call tree.
I think that you refer to not catching user-generated exceptions.
Here the example was given that the caller (a CM method) could decide which
exception to throw whenever "isMonotone" returns false.
My preference is to always throw the same exception (i.e. an instance of
"NonMonoton(e/ic/ous)SequenceException"). However if there is a case where
the caller would want to throw another one, I'd thought that it would be
clearer to catch and rethrow. Well, that's ugly too; in the end, I think we
should stick to "NonMonoton(e/ic/ous)SequenceException". :-)
> In thinking about this, maybe you are correct. The exception could be
> pruned. I am reticent to do this because it looks like exceptions and
> exception reporting are sore subjects on for the developers on this
> project?
Yes. It used to always be "We want all the details". But then there is also
the recent example of "NotPositiveDefiniteMatrixException" where the
"threshold" is considered too much detail...
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] MathUtils.checkOrder
Posted by Greg Sterijevski <gs...@gmail.com>.
I agree with your assessment that having almost identical methods is a pain.
However, without doing this I need to return a very complicated set of
information from isMonotone to be able to construct the exception.
As for catching the exception, I was under the impression that CM code never
catches exceptions, you propagate them upwards on the call tree.
In thinking about this, maybe you are correct. The exception could be
pruned. I am reticent to do this because it looks like exceptions and
exception reporting are sore subjects on for the developers on this
project?
On Thu, Sep 22, 2011 at 8:39 AM, Gilles Sadowski <
gilles@harfang.homelinux.org> wrote:
> On Wed, Sep 21, 2011 at 08:31:00PM -0500, Greg Sterijevski wrote:
> > Any objections to fixing this?
>
> Having a method
> public static boolean isMonotone(double[] val,
> OrderDirection dir,
> boolean strict)
> creates unnecessary duplicate code as it does the same thing as calling
> public static boolean checkOrder(double[] val,
> OrderDirection dir,
> boolean strict,
> boolean abort)
> with "abort" set to "false". This particular case of syntatic sugar does
> not
> warrant the code duplication IMHO.
>
> > On Wed, Sep 21, 2011 at 8:27 PM, Phil Steitz <ph...@gmail.com>
> wrote:
> >
> > > 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.
>
> IMO, the main purpose of the CM utilities is to help the CM developer in
> writing code that behave the same in similar situations across different
> parts of the library. So, the caller should not choose which exception to
> throw because the idea is that "checkOrder" will do what the policy
> requires.
> [If the exception really must be changed, it could be done by explicitly
> catching the "default" exception and rethrowing another one.]
>
> I agree that it is nicer to separate the concerns: ideally "checkOrder"
> should call "isMonotone"; but the problem is the *detailed* exception
> message which reporte the index in the array (where the monotonicity is
> not respected) and the values which are in the wrong order.
>
> Sort of combining the above, we note that a caller that would want to use
> "isMonotone" and throw his own exception, will not be able to recover the
> "detailed information" in order to report it.
> Do you think that the exception message (thus the exception class itself)
> should be simplified? If so, we can indeed separate the concerns _and_
> avoid the code duplication.
>
>
> Gilles
>
> > [...]
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [Math] MathUtils.checkOrder
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Wed, Sep 21, 2011 at 08:31:00PM -0500, Greg Sterijevski wrote:
> Any objections to fixing this?
Having a method
public static boolean isMonotone(double[] val,
OrderDirection dir,
boolean strict)
creates unnecessary duplicate code as it does the same thing as calling
public static boolean checkOrder(double[] val,
OrderDirection dir,
boolean strict,
boolean abort)
with "abort" set to "false". This particular case of syntatic sugar does not
warrant the code duplication IMHO.
> On Wed, Sep 21, 2011 at 8:27 PM, Phil Steitz <ph...@gmail.com> wrote:
>
> > 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.
IMO, the main purpose of the CM utilities is to help the CM developer in
writing code that behave the same in similar situations across different
parts of the library. So, the caller should not choose which exception to
throw because the idea is that "checkOrder" will do what the policy
requires.
[If the exception really must be changed, it could be done by explicitly
catching the "default" exception and rethrowing another one.]
I agree that it is nicer to separate the concerns: ideally "checkOrder"
should call "isMonotone"; but the problem is the *detailed* exception
message which reporte the index in the array (where the monotonicity is
not respected) and the values which are in the wrong order.
Sort of combining the above, we note that a caller that would want to use
"isMonotone" and throw his own exception, will not be able to recover the
"detailed information" in order to report it.
Do you think that the exception message (thus the exception class itself)
should be simplified? If so, we can indeed separate the concerns _and_
avoid the code duplication.
Gilles
> [...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] MathUtils.checkOrder
Posted by Greg Sterijevski <gs...@gmail.com>.
Any objections to fixing this?
On Wed, Sep 21, 2011 at 8:27 PM, Phil Steitz <ph...@gmail.com> wrote:
> 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 <ph...@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
>
>
Re: [Math] MathUtils.checkOrder
Posted by Phil Steitz <ph...@gmail.com>.
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 <ph...@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
Re: [Math] MathUtils.checkOrder
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Wed, Sep 21, 2011 at 08:11:00PM -0500, 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?
You misinterpreted the use of "abort".
Gilles
P.S. The name "checkOrder" is akin to other "check..." methods in CM.
"isMonotone" is nicer but it would be misleading if "abort" is set to
"true".
I guess that you could write "isMonotone" and then "checkOrder" would
call it and, if it returns "false", throw the exception.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] MathUtils.checkOrder
Posted by Greg Sterijevski <gs...@gmail.com>.
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?
On Wed, Sep 21, 2011 at 7:41 PM, Phil Steitz <ph...@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
>
>
Re: [Math] MathUtils.checkOrder
Posted by Phil Steitz <ph...@gmail.com>.
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
Re: [Math] MathUtils.checkOrder
Posted by Greg Sterijevski <gs...@gmail.com>.
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?
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
>
>
Re: [Math] MathUtils.checkOrder
Posted by Greg Sterijevski <gs...@gmail.com>.
The reason I am looking at checkOrder is your suggestion for
UpdatingMultipleLinearRegression, eg checking if the variables are presented
in monotonically increasing order...
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
>
>
Re: [Math] MathUtils.checkOrder
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
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
Re: [Math] MathUtils.checkOrder
Posted by Greg Sterijevski <gs...@gmail.com>.
Meant to say add, not replace. My apologies. -Greg
On Wed, Sep 21, 2011 at 5:05 PM, Greg Sterijevski <gs...@gmail.com>wrote:
> Hello All,
>
> In MathUtils there exists the method:
>
> public static boolean checkOrder(double[] val, OrderDirection dir,
> boolean strict, boolean abort) {
> ...code omitted...
> }
>
>
> I would like to replace it with the method:
>
> public static boolean checkOrder(Comparable[] val, OrderDirection dir,
> boolean strict, boolean abort ){
> Comparable previous = val[0];
> boolean ok = true;
> int max = val.length;
> int comp;
> for (int i = 1; i < max; i++) {
> comp = val[i].compareTo(previous);
> switch (dir) {
> case INCREASING:
> if (strict) {
> if (0 <= comp) {
> ok = false;
> }
> } else {
> if ( comp < 0) {
> ok = false;
> }
> }
> break;
> case DECREASING:
> if (strict) {
> if (comp >= 0) {
> ok = false;
> }
> } else {
> if (comp > 0) {
> ok = false;
> }
> }
> break;
> default:
> // Should never happen.
> throw new IllegalArgumentException();
> }
>
> if (!ok && abort) {
> throw new NonMonotonousSequenceException(val[i], previous,
> i, dir, strict);
> }
> previous = val[i];
> }
>
> return ok;
> }
>
> Would this be acceptable to all?
>
> I would also need to change NonMonotonousSequenceException. It would take
> comparables in the constructor and look like:
>
> public NonMonotonousSequenceException(Comparable wrong,
> Comparable previous,
> int index,
> MathUtils.OrderDirection
> direction,
> boolean strict) {
> super(direction == MathUtils.OrderDirection.INCREASING ?
> (strict ?
> LocalizedFormats.NOT_STRICTLY_INCREASING_SEQUENCE :
> LocalizedFormats.NOT_INCREASING_SEQUENCE) :
> (strict ?
> LocalizedFormats.NOT_STRICTLY_DECREASING_SEQUENCE :
> LocalizedFormats.NOT_DECREASING_SEQUENCE),
> (Number) previous.compareTo(wrong),
> (Number)0, index, index - 1);
>
> this.direction = direction;
> this.strict = strict;
> this.index = index;
> this.previous = null;
> this.previousComp = previous;
> }
>
>
>
> Thoughts?
>
> Thanks!
>
> -Greg
>
>
>
Re: [Math] MathUtils.checkOrder
Posted by Greg Sterijevski <gs...@gmail.com>.
I would not want to remove the current implementation (the one with double[]
as an arg). However, I might want to check lists to make sure that they are
monotonically increasing. I want to avoid writing a checkOrder method for
int[], long[], float[],..., if it is possible. Also, one should be able to
check java.lang.Objects for monotonicity (given they implement Comparable).
Using comparable is (probably) not as efficient as comparing primitives, but
the upside is cleaner code.
On Wed, Sep 21, 2011 at 5:53 PM, Gilles Sadowski <
gilles@harfang.homelinux.org> wrote:
> Hi.
>
> >
> > In MathUtils there exists the method:
> >
> > public static boolean checkOrder(double[] val, OrderDirection dir,
> > boolean strict, boolean abort) {
> > ...code omitted...
> > }
> >
> >
> > I would like to replace it with the method:
> >
> > public static boolean checkOrder(Comparable[] val, OrderDirection dir,
> > boolean strict, boolean abort ){
> > Comparable previous = val[0];
> > boolean ok = true;
> > int max = val.length;
> > int comp;
> > for (int i = 1; i < max; i++) {
> > comp = val[i].compareTo(previous);
> > switch (dir) {
> > case INCREASING:
> > if (strict) {
> > if (0 <= comp) {
> > ok = false;
> > }
> > } else {
> > if ( comp < 0) {
> > ok = false;
> > }
> > }
> > break;
> > case DECREASING:
> > if (strict) {
> > if (comp >= 0) {
> > ok = false;
> > }
> > } else {
> > if (comp > 0) {
> > ok = false;
> > }
> > }
> > break;
> > default:
> > // Should never happen.
> > throw new IllegalArgumentException();
> > }
> >
> > if (!ok && abort) {
> > throw new NonMonotonousSequenceException(val[i],
> previous,
> > i, dir, strict);
> > }
> > previous = val[i];
> > }
> >
> > return ok;
> > }
> >
> > Would this be acceptable to all?
>
> Probably not; we would not be able to call the new method with a "double[]"
> argument. Wherever such a call occurs, we would have to create a "Double[]"
> and copy over the contents, which seems a lot of operations just for the
> sake of a precondition test.
>
> Where do you want to use the new method?
> Anyway, if this method is needed within CM, the obvious possibility is to
> create it (and, unfortunately, almost duplicate the other one).
>
> >
> > I would also need to change NonMonotonousSequenceException. It would take
> > comparables in the constructor and look like:
> >
> > public NonMonotonousSequenceException(Comparable wrong,
> > Comparable previous,
> > int index,
> > MathUtils.OrderDirection
> > direction,
> > boolean strict) {
> > super(direction == MathUtils.OrderDirection.INCREASING ?
> > (strict ?
> > LocalizedFormats.NOT_STRICTLY_INCREASING_SEQUENCE :
> > LocalizedFormats.NOT_INCREASING_SEQUENCE) :
> > (strict ?
> > LocalizedFormats.NOT_STRICTLY_DECREASING_SEQUENCE :
> > LocalizedFormats.NOT_DECREASING_SEQUENCE),
> > (Number) previous.compareTo(wrong),
> > (Number)0, index, index - 1);
> >
> > this.direction = direction;
> > this.strict = strict;
> > this.index = index;
> > this.previous = null;
> > this.previousComp = previous;
> > }
> >
>
> Same here. Better create a new exception that will directly inherit from
> "MathIllegalArgumentException" (instead of "MathIllegalNumberException").
> [I also don't get what are the arguments.]
>
> >
> > [...]
>
>
> Regards,
> Gilles
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [Math] MathUtils.checkOrder
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.
>
> In MathUtils there exists the method:
>
> public static boolean checkOrder(double[] val, OrderDirection dir,
> boolean strict, boolean abort) {
> ...code omitted...
> }
>
>
> I would like to replace it with the method:
>
> public static boolean checkOrder(Comparable[] val, OrderDirection dir,
> boolean strict, boolean abort ){
> Comparable previous = val[0];
> boolean ok = true;
> int max = val.length;
> int comp;
> for (int i = 1; i < max; i++) {
> comp = val[i].compareTo(previous);
> switch (dir) {
> case INCREASING:
> if (strict) {
> if (0 <= comp) {
> ok = false;
> }
> } else {
> if ( comp < 0) {
> ok = false;
> }
> }
> break;
> case DECREASING:
> if (strict) {
> if (comp >= 0) {
> ok = false;
> }
> } else {
> if (comp > 0) {
> ok = false;
> }
> }
> break;
> default:
> // Should never happen.
> throw new IllegalArgumentException();
> }
>
> if (!ok && abort) {
> throw new NonMonotonousSequenceException(val[i], previous,
> i, dir, strict);
> }
> previous = val[i];
> }
>
> return ok;
> }
>
> Would this be acceptable to all?
Probably not; we would not be able to call the new method with a "double[]"
argument. Wherever such a call occurs, we would have to create a "Double[]"
and copy over the contents, which seems a lot of operations just for the
sake of a precondition test.
Where do you want to use the new method?
Anyway, if this method is needed within CM, the obvious possibility is to
create it (and, unfortunately, almost duplicate the other one).
>
> I would also need to change NonMonotonousSequenceException. It would take
> comparables in the constructor and look like:
>
> public NonMonotonousSequenceException(Comparable wrong,
> Comparable previous,
> int index,
> MathUtils.OrderDirection
> direction,
> boolean strict) {
> super(direction == MathUtils.OrderDirection.INCREASING ?
> (strict ?
> LocalizedFormats.NOT_STRICTLY_INCREASING_SEQUENCE :
> LocalizedFormats.NOT_INCREASING_SEQUENCE) :
> (strict ?
> LocalizedFormats.NOT_STRICTLY_DECREASING_SEQUENCE :
> LocalizedFormats.NOT_DECREASING_SEQUENCE),
> (Number) previous.compareTo(wrong),
> (Number)0, index, index - 1);
>
> this.direction = direction;
> this.strict = strict;
> this.index = index;
> this.previous = null;
> this.previousComp = previous;
> }
>
Same here. Better create a new exception that will directly inherit from
"MathIllegalArgumentException" (instead of "MathIllegalNumberException").
[I also don't get what are the arguments.]
>
> [...]
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org