commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: [Math] MathUtils.checkOrder
Date Thu, 22 Sep 2011 13:39:08 GMT
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 <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.

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


Mime
View raw message