On Thu, 2 Oct 2025 07:14:56 GMT, Matthias Baesken <[email protected]> wrote:
>>> > You can use str.isEmpty() here.
>>>
>>> I was actually going for consistency with all of the other optimizations of
>>> this type, which all use a length check. I can change it to `isEmpty` if
>>> you feel strongly about it, though.
>>
>> I think a lot of cases were written before isEmpty() existed so would have
>> used length().
>> isEmpty() might perform a 'tiny' bit better. But that's all.
>>
>>>
>>> > Do we actually call this method with an empty string?
>>>
>>> I do think it's possible, e.g. from `SunGraphics2D.drawString(String, int,
>>> int)` and `SunGraphics2D.drawString(String, float, float)` when the font
>>> doesn't have layout attributes, or a few other places when
>>> `OutlineTextRenderer.drawChars(...)` delegates to
>>> `OutlineTextRenderer.drawString(...)`.
>>
>> I agree.
>> Even if we added some checks there, it would require work to prove that
>> there's no way to get here with an empty string.
>> I see no harm in the check here, especially if we aren't 100% confident.
>
>> > You can use str.isEmpty() here.
>>
>> I was actually going for consistency with all of the other optimizations of
>> this type, which all use a length check. I can change it to `isEmpty` if you
>> feel strongly about it, though.
>>
>
> Hi, you replaced this strange `''".equals` check ; is this because it looks
> not nice compared to length or isEmpty or for other reasons ? I ask because
> there are quite a few of those checks in the codebase (below are only some of
> them) .
>
>
>
> src/java.base/share/classes/java/lang/runtime/ObjectMethods.java:416:
> List<String> nameList = "".equals(names) ? List.of() :
> List.of(names.split(";"));
> src/java.base/share/classes/java/net/NetworkInterface.java:230: return
> "".equals(displayName) ? null : displayName;
> src/java.base/share/classes/java/net/SocketPermission.java:885: if
> (this.wildcard && "".equals(this.cname))
> src/java.base/share/classes/java/security/CodeSource.java:466: if
> (("".equals(thisHost) || "localhost".equals(thisHost)) &&
> src/java.base/share/classes/java/security/CodeSource.java:467:
> ("".equals(thatHost) || "localhost".equals(thatHost))) {
> src/java.base/share/classes/java/text/CompactNumberFormat.java:2546:
> !"".equals(compactPatterns[index])) { // ignore empty pattern
> src/java.base/share/classes/sun/security/tools/keytool/Main.java:931:
> if ("".equals(dest)) {
> src/java.base/share/classes/sun/security/tools/keytool/Main.java:939:
> if ("".equals(alias)) {
> src/java.base/share/classes/sun/security/tools/keytool/Main.java:2510:
> if ("".equals(newAlias)) {
> src/java.base/share/classes/sun/security/util/SecurityProperties.java:155:
> if ("".equals(rawPropVal)) {
> src/java.base/share/classes/sun/util/locale/provider/LocaleResources.java:604:
> } else if ("".equals(pattern)) {
> src/java.desktop/macosx/classes/com/apple/laf/AquaTextFieldSearch.java:246:
> if (!text.hasFocus() && "".equals(text.getText())) {
> src/java.desktop/macosx/classes/com/apple/laf/AquaTextFieldSearch.java:290:
> button.setVisible(!"".equals(text.getText()));
> src/java.desktop/share/classes/com/sun/media/sound/JDK13Services.java:175:
> if ("".equals(value)) {
> src/java.desktop/share/classes/java/awt/FileDialog.java:153:
> fileDialog.file = ("".equals(file)) ? null : file;
> src/java.desktop/share/classes/java/awt/FileDialog.java:156:
> fileDialog.dir = ("".equals(directory)) ? null : directory;
> src/java.des...
@MBaesken We decided to keep the check as a pure optimization (not just a
safety check required to avoid an exception) after seeing that there are other
similar checks (as optimizations) in the code base, and used this specific
formulation simply for consistency with all of the other existing equivalent
checks (see list here:
https://github.com/openjdk/jdk/pull/26947#issuecomment-3329884998).
-------------
PR Comment: https://git.openjdk.org/jdk/pull/27523#issuecomment-3360249841
