Proposed API for JEP 259: Stack-Walking API

[Paul Sandoz]

> On 4 Nov 2015, at 04:50, Mandy Chung <mandy.chung at oracle.com> wrote:
> 
> 
>> On Nov 3, 2015, at 3:28 AM, Paul Sandoz <paul.sandoz at oracle.com> wrote:
>> 
>> Hi Mandy,
>> 
>> This is very nice work.
>> 
>> Comments below, mostly minor stuff.
>> 
> 
> Thanks for the review.
> 
> I fixed most of the comments below.   One question:
> 
> Is the name “StackStream" inappropriate and its subtypes?  I prefer StackStream to StackTraverser.
> 

I think using Stream in the name gives the wrong impression. It’s not a subtype of j.u.Stream, it’s the thing that is used as a source for a Stream, in some cases.

IIUC it’s really two things, a form of Spliterator that supports sequential traversal of stack elements via tryAdvanced/forEachRemaining, and a thing to directly query stack elements.


> 
>> 
>> StackStream
>> —
>> 
>> Suggest using something other than Stream in the name is this is really about Spliterator and traversal e.g. perhaps StackTraverser. Same for sub-types of StackStream.
>> 
>> 494     @Override
>> 495     public void forEachRemaining(Consumer<? super StackFrame> action) {
>> 496         throw new UnsupportedOperationException("Specialized StackWalker");
>> 497     }
>> 498
>> 499     @Override
>> 500     public boolean tryAdvance(Consumer<? super StackFrame> action) {
>> 501         throw new UnsupportedOperationException("Specialized StackWalker");
>> 502     }
>> 
>> Is this required?
>> 
> 
> CallerClassStream and StackTrace don’t support to use with Stream API for performance.

Then perhaps those classes should implement those methods to throw USO? or there should be a sub-type which indicates Spliterator traversal is not supported? Then i think it becomes much clearer on what sub-types support what contract.

Or maybe the Spliterator can be pushed down to sub-types that are used as the source for a Stream? (I have not re-looked into the details to how awkward this might become.)


> 
>> 
>> 238     private final List<? extends WalkerOption> options;
>> 
>> Do you require the bound here?
>> 
> 
> Sorry WalkerOption has been removed.  I only updated the API but not webrev.00.
> 

Ok, thats good!


>> 
>> 241     /**
>> 242      * Gets a {@code StackWalker} instance.
>> 243      *
>> 244      * <p> This {@code StackWalker} obtains the method name and the class name
>> 245      * with all {@linkplain StackWalker.Option#SHOW_HIDDEN_FRAMES hidden frames}
>> 246      * skipped.
>> 247      */
>> 248     public StackWalker() {
>> 
>> s/Gets/Creates
>> 
>> Same for other constructor methods.
>> 
>> 
>> 252     /**
>> 253      * Gets a {@code StackWalker} instance with the given options specifying
>> 254      * the stack frame information it can access.  If no option is specified, this
>> 255      * {@code StackWalker} obtains the method name and the class name
>> 256      * with all {@linkplain StackWalker.Option#SHOW_HIDDEN_FRAMES hidden frames}
>> 257      * skipped.
>> 
>> If no options are given…
>> 
>> It might be easier to state as if the options were {CLASS_REFERENCE, SHOW_REFLECT_FRAMES}, perhaps it depends if the default is someone under specified i.e. is it guaranteed that the option set will be the complement of the single set SHOW_HIDDEN_FRAMES?
> 
> In fact the default can’t be expressed with the current options.  This is a good point though and I have been considering adding SHOW_METHOD_INFO which will be the default.   I’ll update the API.
> 

Ok.


>> 
>> I guess any unrecognized options are ignored?
>> 
> 
> Now it takes only Option enums.
> 

That’s better.


> 
>> 
>> 430     /**
>> 431      * Gets the {@code Class} object of the caller invoking the method
>> 432      * that calls this {@code getCallerClass} method.
>> 433      *
>> 434      * <p>This is equivalent to calling
>> 435      * <blockquote>
>> 436      * <pre>{@code walk((s) ->}
>> 
>> } -> {
>> 
>> 437      *     s.map(StackFrame::getDeclaringClass)
>> 438      *      .skip(2)
>> 439      *      .findFirst()).orElse(Thread.currentThread().getClass());
>> 440      * </pre></blockquote>
>> 
>> 
>> 441      *
>> 442      * <p>For example, {@code Util::getResourceBundle} loads a resource bundle on behalf
>> 443      * of the caller.  It calls this {@code getCallerClass} method to find the method
>> 444      * calling {@code Util::getResourceBundle} and use the caller's class loader to
>> 445      * load the resource bundle. The caller class in this example is the {@code Foo} class.
>> 
>> 
>> use @apiNote
>> 
>> 
>> BatchSizeMapper
>> —
>> 
>> As usual i always advice using testng and data providers, up to you.
> 
> I started this one with TestNG but this data provider requires the method to return Object[][].  I automatically resorted to old-school test :)
> 

:-) it’s more of an annoyance, i often compose using a list then convert. I can help out if you like.

IIRC it may also possible to return Iterable<Object[]> or Iterator<Object[]> i cannot remember, but don’t expect the TestNG documentation to tell you that!, you need to look into the source code to work that out.

Paul.