commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Sterijevski <gsterijev...@gmail.com>
Subject Re: [Math] MathUtils.checkOrder
Date Thu, 22 Sep 2011 01:31:00 GMT
Any objections to fixing this?

On Wed, Sep 21, 2011 at 8:27 PM, Phil Steitz <phil.steitz@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 <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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message