Hello Luc. > > > >> [...] > >>> So, what is the lesson from having this case? Is the whole method a > >>> duplicate > >>> that should be replace by a call to something in "linear"? Or is it a > >>> proof > >>> that some exceptions might be appropriately used outside of "linear"? > >> > >> I think the method should be moved to a new > >> TruncatedCholeskyDecomposition or RectangularCholeskyDecomposition. I > >> don't think we can add it has a new implementation or it should be added > >> to the existing CholeskyDecomposition because the decomposition matrix > >> shape is not triangular. > > > > If you are going to create new classes in "linear" so that the current CM > > code only throws "...MatrixException"s from inside that package, I'm not > > going argue anymore about moving those exceptions to "linear". But, if this > > situation arises again, then we will create a incompatibility by moving the > > exception from "linear" to "exception"... That's why I think that it's safer > > (from a stability point-of-view) to have them all in "exception". > > I don't think this will happen very often. > So let's create the new classes (I'll open a Jira issue for that) and > move the linear algebra related exceptions back to the linear package.
I'll do the move. Please, don't touch anything in the "exception" package at the moment: I'm currently implementing the "map" change. > >>>>> [...] > >>>> > >>>> For example to have a single point where to put breakpoints for > >>>> debugging. I used that a lot as such exceptions mainly occur during > >>>> development phase. > >>> > >>> Thus, IIUC, this is a developer feature. > >>> I understand the convenience, but I wouldn't trade code consistency for > >>> it. > >>> I'd rather keep "NullArgumentException" (or a "MathNullPointerException" > >>> if > >>> we want it to extend the standard NPE) so that you can put your breakpoint > >>> in the constructor of that exception. Thus you have the convenience > >>> without > >>> the code asymmetry. > >> > >> OK for MathNullPointerException. > > > > With a "checkNotNull" function, it should not be necessary. > > But I didn't quite understood your reasoning concerning such a method and > > whether it's OK to implement or not (cf. below). > > > >>> > >>>>> > >>>>> Not only is this inconvenient, it is also dangerous: I was bitten more > >>>>> than > >>>>> once writing this: > >>>>> --- > >>>>> MathRuntimeException.createNullPointerException(); > >>>>> --- > >>>>> which the compiler happily accepted. > >>>> > >>>> Of course, as it would have accepted: > >>>> > >>>> new NullPointerException(); > >>>> > >>>> Unfortunately, we cannot put the throw inside the method. Trying to do > >>>> this prevent the optimizer to wor properly as it does not know in the > >>>> caller that the method would never return, and it brings lots of > >>>> warnings from code checking tools for the same reason. > >>> > >>> The method alternative looses much of its appeal because it doesn't > >>> perform > >>> the complete action ("throw" included), as I had initially assumed when > >>> wrongly leaving out the "throw" statement. I wouldn't have written > >>> new NullPointerException(); > >>> as, to me, it would have _obviously_ looked wrong. > >>> > >>> I don't understand the problem with the "optimizer". There are several > >>> methods in CM that seem to perform similarly (i.e. they check something > >>> and > >>> throw an exception). An example is "checkOrder" in "MathUtils". > >>> In fact we could add a "checkNotNull" in "MathUtils": > >>> --- > >>> public checkNotNull(Object obj) { > >>> if (obj == null) { > >>> throw new NullPointerException(); > >>> } > >>> } > >>> --- > >>> Wouldn't that satisfy all needs (ability to set a breakpoint and no > >>> factory > >>> methods)? > >> > >> No the check may succeed or not, so sometimes these methods do return. > > > > I'd say that they return most of the time, and that's fortunate... :-) > > Sorry, I misunderstood what you were writing. I was still talking about > the changing the createXxxException (which only creates) into > throwXxxException (which creates and throws), not about > checkXxxCondition (which checks, creates and throws). > > > > >> What I had in mind was something like: > >> > >> public static void throwNPE() { > >> throw new MySpecialNPE extends NullPointerException() { > >> // my special code here > >> }; > >> } > >> > >> Such a method *never* returns but on the caller side, the compiler, and > >> the checking tools don't know it. Typically when you have code like: > >> > >> int var; > >> if (obj != null) { > >> var = obj.getVar(); > >> } else { > >> throwNPE(); > >> } > >> > >> // use var > >> > >> Then tools will complain that var may not be initialized, which is > >> false. However it is only because throwNPE never returns that it is false. > > > > Personally, I don't like this kind of syntax. The good programming rule is > > to assign at declaration. [Unfortunately, one is sometimes forced to write > > in this way and most often than not because of checked exceptions!] > > > >> Another way to put it that would probably work is: > >> > >> public static void checkNonNull(Object o) { > >> if (o == null) { > >> throw new MySpecialNPE extends NullPointerException() { > >> // my special code here > >> }; > >> } > >> } > > > > How is this different from my version above? > > It's the same, I simply get confused. So go for it, but including the > specialization and localization part that was in the former > createxxxException. So basically we add an if (obj == null) wrapper > aroud the 2.1 code. So to be sure we say the same thing, you'd like to have (in "MathUtils"): --- public static void checkNotNull(Object o, Localizable pattern, Object ... args) { if (o == null) { throw new NullArgumentException(pattern, args); } } --- Where we keep the _non-standard_ "NullArgumentException". [In line with what we agreed on before: That it made sense (for the various reasons we dicussed at length) to have a singly rooted hierarchy and thus depart from the standard exceptions sub-classes.] And I'll also add --- public static void checkNotNull(Object o) { if (o == null) { throw new NullArgumentException(); } } --- for those developers who will be content with the default message ("null is not allowed"). OK? Regards, Gilles --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org