----- Mail original ----- > De: "Mandy Chung" <mandy.ch...@oracle.com> > À: "Remi Forax" <fo...@univ-mlv.fr> > Cc: core-libs-dev@openjdk.java.net > Envoyé: Samedi 31 Octobre 2015 23:59:01 > Objet: Re: Proposed API for JEP 259: Stack-Walking API > > > > On Oct 31, 2015, at 11:29 AM, Remi Forax <fo...@univ-mlv.fr> wrote: > > > > Hi Mandy, > > I've crawled the code and the documentation again. > > > > In the doc and in the code, a lambda with one parameter doesn't require > > parenthesis around the parameter, > > (s) -> s.doSomething() > > should be > > s -> s.doSomething(). > > > > Oh right. (It didn’t look right to me but didn’t pay attention to it). > > > > > In the doc of StackWalker, > > in the first example, the local variable 'frame' should be named > > ‘callerClass' > > > > Fixed > > > > In the doc of getCallerClass(), > > the first example do a skip(2) which i believe is not necessary anymore, > > It has to skip two frames. Use the second example, the stack looks like > this: > > StackWalk::getCallerClass > Util::getResourceBundle > Foo::init > : > :
Ok, get it ! > > > also instead of Optional.orElse, orElseGet is better because it avoids to > > evaluate > > Thread.currentThread().getClass() if not necessary. > > > > So the example should be: > > walk(s -> s.map(StackFrame::getDeclaringClass) > > .findFirst()).orElseGet(() -> > > Thread.currentThread().getClass()); > > > > This would return Util.class instead of Foo.class > > > In the second example, the field walker should be declared 'final’. > > Sure. Fixed. > > > > > And as i said earlier, the signature of walk() is: > > <T> T walk(Function<? super Stream<StackWalker.StackFrame>, ? extends T> > > function, IntUnaryOperator batchSizeMapper) > > > > I was pondering it and that’s why not changed in the last update. I agree > with the upper bounded wildcard "? extends T” for the return type of the > function. > > But for the function’s input argument, can you help me understand why it > should take "? super Stream<StackWalker.StackFrame>”? Is it useful to have > function accepting supertype of Stream<StackFrame> rather than > Stream<StackFrame>? VM should be the only source producing this StackFrame > stream. currently this code doesn't compile, and should IMO, Function<BaseStream<StackWalker.StackFrame>, Void> fun = null; stackWalker.walk(fun); > > On the other hand, I wonder if the stream argument should be Stream<? extends > StackFrame> that may allow future implementation change. > > <T> T walk(Function<Stream<? extends StackWalker.StackFrame>, ? extends T> > function, …) A stream of ? extends StackFrame means that you can not call any methods of Stream<T> that takes a T as parameter because the compiler can not enforce that the type is correct, so you can not call neither reduce(BinaryOperator<T> ) nor reduce(T, BinaryOperator<T>). If you want to be able to send subtypes of StackFrame and know that at compile time, it's better to parameterized the StackWalker class by the type of StackFrame. > > Mandy Rémi
