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] Explanations for MATH-459
Date Mon, 17 Jan 2011 13:02:07 GMT
Hello Michael.

On Mon, Jan 17, 2011 at 02:38:42AM +0200, Michael Giannakopoulos wrote:
> Well, as far as this issue is concerned i have made lots of changes to the
> current code in the repository. In order to be sure that these changes is
> what you are expecting i would like to ask some questions... First of all,
> we want the class "MathRuntimeException" in the package
> 'org.apache.commons.math' to be deprecated so we don't mind if some of the
> new exceptions that are going to be used are based on the class
> "MathRuntimeException" located in the package
> 'org.apache.commons.math.exception'.

It's not just that "we don't mind": *all* the exceptions in Commons Math
must inherit from (the new) "MathRuntimeException".

> For example, an exception that is
> frequently used in the code is
> "MathRuntimeException.createIllegalArgumentException(...)" [see class
> "ResizableDoubleArray" in package 'org.apache.commons.math.util']. If the
> exception above is going to be replaced by "MathIllegalArgumentException"
> which is located in 'org.apache.commons.math.exception' then i suppose that
> our goal has achieved.

No, that would be too easy ;-)
Where, previously, the specifics of the problem were only referred to by an
"enum", we now want that as much as possible is conveyed through the type of
exception. So, whenever there exists a specific exception, it must be used
instead of the more general "MathIllegalArgumentException".
E.g. for the class "ResizableDoubleArray", there is (at line 388)
---CUT---
  if (contraction < expansion) {
    throw MathRuntimeException.createIllegalArgumentException(LocalizedFormats.CONTRACTION_CRITERIA_SMALLER_THAN_EXPANSION_FACTOR,
                                                              contraction, expansion);
  }
---CUT---
This should be replaced by
---CUT---
  if (contraction < expansion) {
    throw new NumberIsTooSmallException(LocalizedFormats.CONTRACTION,
                                        contraction,
                                        expansion,
                                        true);
  }
---CUT---
Caveat here (as explained in my previous message), the "enum" element
"CONTRACTION" does not exist yet. [I would suggest that you first make the
changes when you don't have to create new ones so that we can examine how we
can best limit the number of them.]

A more obvious example is (at line 572)
---CUT---
    public synchronized double getElement(int index) {
        if (index >= numElements) {
            throw MathRuntimeException.createArrayIndexOutOfBoundsException(
                    LocalizedFormats.INDEX_LARGER_THAN_MAX,
                    index, numElements - 1);
        } else if (index >= 0) {
            return internalArray[startIndex + index];
        } else {
            throw MathRuntimeException.createArrayIndexOutOfBoundsException(
                    LocalizedFormats.CANNOT_RETRIEVE_AT_NEGATIVE_INDEX,
                    index);
        }
    }
---CUT---
This can simply become
---CUT---
public synchronized double getElement(int index) {
  if (index < 0 ||
      index >= numElements) {
    throw new OutOfRangeException(index, 0, numElements - 1);
  }

  return internalArray[startIndex + index];
}
---CUT---
In this case, the simplification is warranted (IMO) because the specific
"enum" states the obvious and is redundant with the semantics of the
"OutOfRangeException" type.

> Can someone verify this to me, so as to be sure that
> i'm on the right way?

I strongly suggest that you do modifications step-by-step, and provide your
first "diff" files on this list so that people can possibly put you on the
right track with concrete remarks.

> Also, as Luc has said to one mail there are some
> exceptions like "MathRuntimeException.createInternalError(...)" [see class
> "RandomDataImpl" in 'org.apache.commons.math.random'] that can be replaced
> by the default exceptions that java language provides (in our example the
> aforementioned exception can be replaced with "InternalError" exception).

No. We decided (cf. older threads on this ML and JIRA issues) that we won't
use the standard exceptions. All the CM exceptions hierarchy is rooted at
"MathRuntimeException" (that inherits from the standard "RuntimeException").
[E.g. for the above case, Luc will create a "MathInternalError".]

> [...]


Best regards,
Gilles

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


Mime
View raw message