RFR: 8210683: Search result display order reversed for overloaded entries

Discussion:

Hannes Wallnöfer

2018-10-10 09:16:29 UTC

Please review:

Bug: https://bugs.openjdk.java.net/browse/JDK-8210683
Webrev: http://cr.openjdk.java.net/~hannesw/8210683/webrev.00/

Docs generated with patch applied:
http://cr.openjdk.java.net/~hannesw/8210683/api/

Note that the new behaviour is to put the var-args signature last, whereas before var-args was in first position (even before the no-args signature).

Thanks,
Hannes

Jonathan Gibbons

2018-10-11 22:36:01 UTC

Permalink

Nice!

FWIW, the style in langtools is to avoid the language "assert"
mechanism, because it is too difficult
to control if/when assertions might be enabled. javac provides an
Assert class, but that is not
available here, so in this case I would either remove the assert, if you
think the check is not that
important, or else use instanceof, if you think it is important.

-- Jon

Post by Hannes WallnÃ¶fer
Bug: https://bugs.openjdk.java.net/browse/JDK-8210683
Webrev: http://cr.openjdk.java.net/~hannesw/8210683/webrev.00/
http://cr.openjdk.java.net/~hannesw/8210683/api/
Note that the new behaviour is to put the var-args signature last, whereas before var-args was in first position (even before the no-args signature).
Thanks,
Hannes

Hannes Wallnöfer

2018-10-15 13:44:13 UTC

Permalink

Thanks for the review, Jon.

My rationale was that RuleBasedCollator is the only Collator subclass in the JDK. But thinking about it some more, I think adding an instanceof condition will be a more robust choice. So here’s a new webrev with that change:

http://cr.openjdk.java.net/~hannesw/8210683/webrev.01/

Hannes

Nice!
FWIW, the style in langtools is to avoid the language "assert" mechanism, because it is too difficult
to control if/when assertions might be enabled. javac provides an Assert class, but that is not
available here, so in this case I would either remove the assert, if you think the check is not that
important, or else use instanceof, if you think it is important.
-- Jon

Jonathan Gibbons

2018-10-16 03:41:30 UTC

Permalink

Post by Hannes WallnÃ¶fer
Thanks for the review, Jon.
http://cr.openjdk.java.net/~hannesw/8210683/webrev.01/
Hannes

Nice!
FWIW, the style in langtools is to avoid the language "assert" mechanism, because it is too difficult
to control if/when assertions might be enabled. javac provides an Assert class, but that is not
available here, so in this case I would either remove the assert, if you think the check is not that
important, or else use instanceof, if you think it is important.
-- Jon