On 5/2/11 1:38 AM, Gilles Sadowski wrote: > On Sun, May 01, 2011 at 03:18:20PM -0700, Phil Steitz wrote: >> On 5/1/11 2:29 PM, Gilles Sadowski wrote: >>> On Sun, May 01, 2011 at 08:11:00AM -0700, Phil Steitz wrote: >>>> On 5/1/11 3:48 AM, Gilles Sadowski wrote: >>>>> On Sat, Apr 30, 2011 at 10:53:30PM -0700, Phil Steitz wrote: >>>>>> On 4/30/11 4:33 PM, Gilles Sadowski wrote: >>>>>>> On Sat, Apr 30, 2011 at 09:10:08AM -0700, Phil Steitz wrote: >>>>>>>> Converting some of my code to use trunk, I discovered that the >>>>>>>> binomialCoefficient methods no longer throw IllegalArgumentException >>>>>>>> when parameters are invalid. >>>>>>> The consensus was a singly rooted hierarchy ("MathRuntimeException"). >>>>>>> The advantage being commonly agreed on was to offer the "map" >>>>>>> functionality >>>>>>> for adding messages and context information. >>>>>> I guess I misunderstood and after really seeing the consequences in >>>>>> my own code, I am going to have to ask that we reopen that >>>>>> discussion - i.e., I would like us to throw IAE and other standard >>>>>> exceptions where appropriate, as in this case, as we have up to and >>>>>> through 2.2. I know I said before that I did not see this as worth >>>>>> arguing about, but I really think this change is a bad API design >>>>>> decision that will cause needless hassle and surprising RTEs for >>>>>> users upgrading to the new version. >>>>> I'm astonished, and for the time being, will refrain from other comments. >>>>> >>>>>>>> The javadoc asserts that >>>>>>>> MathIllegalArgumentException will be thrown in these cases, but that >>>>>>>> is not correct, >>>>>>> I don't understand; the code for "checkBinomial" can throw >>>>>>> "NumberIsTooLargeException" or "NotPositiveException" and they are >>>>>>> subclasses of "MathIllegalArgumentException". >>>>>>> >>>>>> Sorry. my mistake. >>>>>>>> since what is actually thrown now can differ >>>>>>>> depending on the parameter problem >>>>>>> That's a feature, naturally: Different problem, different exception. >>>>>>> >>>>>> That's where I disagree. I see zero value added and in fact >>>>>> confusing complexity introduced by these exceptions. When you ask >>>>>> for B(n,k) where k is not less than or equal to n, a standard IAE >>>>>> with a message that says precisely that (which the current message >>>>>> does) is clear and *better* that a "NumberIsTooLargeException". >>>>>> What number? I guess it must be k? To figure it out you have to >>>>>> look at the exception message, which is *exactly the same message* >>>>>> that the old code reported. If we really think we need to >>>>>> specialize and report different exceptions for every precondition >>>>>> violation (which I do not agree with), then these exceptions should >>>>>> be meaningful in the context of the API that is using them. So >>>>>> here, for example, we would have to throw something like >>>>>> "CombinationSizeTooLargeForSetException." >>>>> Then, we do _not_ disagree _now_. From the start, I stated that a >>>>> consistent >>>>> design would be to define a specific exception for each specific that must >>>>> be reported, especially if it must contain complex functionality like >>>>> localization. >>>>> IIRC, either you or Luc (or both) did not want a large number of >>>>> exceptions. >>>>> To keep the number down, we reuse less specific exception types (like >>>>> "NumberIsTooLargeException" in several contexts) and rely on the >>>>> message(s) >>>>> for context information. Nothing lost from the previous situation (when >>>>> one >>>>> *had* to rely solely on the message)! >>>> But in this case, my opinion is that IAE is just fine - there is >>>> nothing more "specific" to communicate unless you want to define >>>> something meaningful in the context of the API. "NumberIsTooLarge" >>>> has no value here. >>> I don't agree. This is the concept that describes the problem: indeed, the >>> precondition is that "k" must be smaller than "n". >>> This has the same value as the "out of range" concept where you give the >>> value of some parameter and the values of the bounds. >>> >> No, it is actually a different concept. In mathematical terms, it >> is a binary relation that is failing here: k < n. What >> "NumberIsTooLarge" (or "NumberIsTooSmall" which could also be >> applied here, to n instead of k) conveys is unary. > I repeat that I'd be fine with adding an exception that would retain > the binary relationship. If you think it is overkill, fine too. But that > does not make "NumberIsTooLargeException" useless; it is only not precise > enough in this case to fully describe the mathematical situation. From a > programming viewpoint, it is IMO a good compromise: It says something about > how the precondition was tested: > --- > if (k > n) { > throw new NumberIsTooLargeException(k, n); > } > --- > >>>> As I said, if we feel we need to make this >>>> particular exception due to precondition violation more precise, we >>>> would need to define an exception that refers to subset relation >>>> size or somesuch, which I don't see as necessary or valuable. >>> In principle, I've nothing against devising a deeper hierarchy that >>> would make the type of the exception refer to the exact problem. However, >>> there would indeed be not much practical value added if all use cases are >>> about letting the exception bubble upwards until it aborts the program (at >>> which point a human will read the error mesage). >> Sounds like you are arguing here to just leave it as IAE, which is >> fine by me. > No, see below. > >>>>> To answer your question above: No, you don't have to "guess" which number >>>>> is >>>>> too large; there is an accessor "getArgument()" that returns the number >>>>> that >>>>> triggered the exception and another "getMax()" that informs you about the >>>>> maximum allowed number. >>>> No, all that is reported is the *value*. >>> Well, of course: CM is a numerical library, not a symbolic one. >>> >>>> To make this actually >>>> work, you would have to do something like store and report the >>>> formal parameter name. I don't see the point in this in general >>> Me neither. >>> >> Then I would claim "NumberIsTooLarge" and "NumberIsTooSmall" provide >> no value. You need to look at the exception message, which >> thankfully we have preserved in this case, to understand what the >> actual problem is. The abstraction itself is worthless at least in >> this case, IMO. >> >>>> and >>>> certainly not in this case, as what is problematic is the order >>>> relation among the two parameters - not one is "too small" or the >>>> other is "too large" but that they violate the stated preconditions >>>> of the method. >>> I really don't understand what bothers you! >>> The precondition >>> k <= n >>> means that if k > n, then the *value* of k is too large. >>> You can elaborate on the context if you wish, but that does not change the >>> basic problem. >> You are missing the point, stated above. The problem is that we >> have a precondition that is a *relation* between the two parameters >> and that precondition has been violated. Throwing in a >> context-unaware unary predicate that says "one of the parameters is >> too large" adds nothing to precise the problem. > No, in my view, you are missing my many points. > I understand your point about binary relation but, I repeat, CM is not aimed > to be a faithful representation of all mathematical concepts; it is aimed at > solving problems numerically. And to aid debugging when something goes wrong, > Java provides a mechanism called "exceptions". > Concerning the abstraction "number is too large", I find it useful just for > that. Exactly as "out of range" gives a value plus two other values, claiming > that the first one is not in the range defined by the other two. > > If you want that CM throws subtypes of IAE when a precondition fails, that's > fine with me. It's not fine with me to throw a plain IAE because > (1) that is not what we agreed on earlier to mitigate the ugliness (in my > view) of having localization an integral part of CM, This has nothing to do with messages. The "NumberIsTooLarge" thing still carries *exactly the same message* which is what is really relevant to the caller. > (2) it will not provide the "rich context" feature, Look carefully at how the code now appears *from the standpoint of the caller* reading the javadoc. There is nothing useful added as the exception conveys nothing meaningful beyond IAE. > (3) it does not provide the flexibility that an application programmer might > want for customizing an error message. Don't follow this.
In any case, I give up. I can see we are getting nowhere here. Phil > > Gilles > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org