On Jan 11, 2016, at 7:04 AM, Paul Sandoz <paul.san...@oracle.com> wrote:
> 
> When the new range check methods Object.check* were added the exception 
> reporting was a little vague and lossy, but i got a CCC pass to revisit later 
> on, and John nudged me to sort this out, so here is another more reflective 
> approach:
> 
>  
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8146458-checkIndex-exception-reporting/webrev/
>  
> <http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8146458-checkIndex-exception-reporting/webrev/>
>  
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8146458-checkIndex-exception-reporting/specdiff/overview-summary.html
>  
> <http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8146458-checkIndex-exception-reporting/specdiff/overview-summary.html>

I like it.  (No surprise there.)

In the new API point the term "mapper", and "exception mapping function" in the 
existing
API points, doesn't really convey much  to me.

The main cog. diss. is that, at the time the EMF is called, there is no 
exception created yet,
although it is true that an exceptional condition has been identifier.  But the 
exception thrown
is the first to be created; it is not "mapped" from a previous exception.

So I'd prefer "factory", personally:  "factory", "exception factory [function]".
A T-factory maps () to T, while a T-mapper maps (T) to T.

> ...
> 
> The two int arg constructors added to Array/String/IndexOutOfBoundsException 
> have been removed.

Yes; that pays for the new API point, IMO.  Very nice.

One nit:

-+        // Switch to default if fewer arguments than required are supplied
-+        switch ((args.size() < argSize) ? "" : checkKind) {
++        // Switch to default if wrong number of arguments is supplied
++        switch ((args.size() != argSize) ? "" : checkKind) {

That is, excess values should show up, so they can be debugged.
(Unless you have a use case in mind?)

Would you mind including a test case which exercises, say,
ArrayIndexOutOfBoundsException::new?  Might as well
make sure those wires connect properly; test the class of
the thrown exception as well as the message.

> I did ponder about adding a new constructors with String, List<Integer> 
> arguments. I could still do that, but that does spread out the notion beyond 
> that of Objects and i prefer that it be contained.

Nope.  Keep this thing localized.

Reviewed, if you need a reviewer.

— John

Reply via email to