-
Notifications
You must be signed in to change notification settings - Fork 867
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rename ServerSpanNaming to HttpRouteHolder #5211
Rename ServerSpanNaming to HttpRouteHolder #5211
Conversation
|
||
/** An interface for getting the {@code http.route} attribute. */ | ||
@FunctionalInterface | ||
public interface HttpRouteGetter2<T, U> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not obvious to me that it's better but as a reminder, the JDK idiom is to use Bi
, not 2
, e.g. BiFunction
BiConsumer
. I guess here our naming is constrained by having a fixed Context
param along with the arbitrary ones
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That being said, I guess having these types is to avoid capturing lambdas, but are we sure the performance improvement is really enough to avoid the simpler Supplier<String>
or Function<Context, String>
? With inlining and such it's always hard for me to really understand how bad a capturing lambda is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not obvious to me that it's better but as a reminder, the JDK idiom is to use
Bi
, not2
, e.g.BiFunction
BiConsumer
Changed it HttpRouteBiGetter
(because of ToDoubleBiFunction
in the JDK)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With inlining and such it's always hard for me to really understand how bad a capturing lambda is
I opened #5215 (targeted to Stable API project) to make sure that this is an optimization worth keeping
* Rename ServerSpanNaming to HttpRouteHolder * HttpRouteBiGetter
... and
ServerSpanNameSupplier
/ServerSpanNameTwoArgSupplier
toHttpRouteGetter
/HttpRouteGetter2
- I used the functional libs/languages as an inspiration here, they usually have interfaces with names likeFunction2
(e.g. Kotlin).Another part of #442