-
Notifications
You must be signed in to change notification settings - Fork 383
Add emulation for Java 9 additions to Objects class #10107
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Thomas Broyer <[email protected]>
5c9ca6b
to
9b2cac2
Compare
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.
LGTM but would be good to have a second pair of eyes
|
||
public static int checkIndex(int index, int length) { | ||
if (index < 0 || index >= length) { | ||
throw new IndexOutOfBoundsException("Index " + index + " out of bounds for length " + length); |
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.
I'm unsure about this exception message. Can't remember if it'll be pruned in production builds (in which case this is OK) or not (in which case we should just throw the exception without message)
Alternatively, we could emulate the IndexOufOfBoundsException(int) constructor and use it here.
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.
I think if we are exposing user methods to call, we should leave it up to them to prune or not - GWT might prune null checks inside of collections and such if directed, but we wouldn't want to actually rewrite if (foo == null)
etc, and likewise shouldn't optimize these out?
EDIT: you're specifically talking about the error message being pruned, while I'm speaking more broadly, the runtime error at all, and the expected error message (which the developer explicitly wants).
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.
The information should be transported to the user. Some months ago I came across an IOOBE in GWT which did not provide any information and it was annoying.
GWT already has a bunch of different exception messages for IOOBE so I am fine with the text.
I guess we would save more if GWT would provide some internal ctors or factory methods in IOOBE each providing a fixed, short error message and then use these ctors everywhere to get rid of all the different messages.
|
||
public static int checkIndex(int index, int length) { | ||
if (index < 0 || index >= length) { | ||
throw new IndexOutOfBoundsException("Index " + index + " out of bounds for length " + length); |
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.
The information should be transported to the user. Some months ago I came across an IOOBE in GWT which did not provide any information and it was annoying.
GWT already has a bunch of different exception messages for IOOBE so I am fine with the text.
I guess we would save more if GWT would provide some internal ctors or factory methods in IOOBE each providing a fixed, short error message and then use these ctors everywhere to get rid of all the different messages.
Add emulation for Objects methods
Does not cover the Java 16 versions of these that accept
long
instead ofint
.