On Tue, 21 Apr 2026 10:57:40 GMT, Alan Bateman <[email protected]> wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add `ofString` static factory method
>
> src/java.base/share/classes/javax/net/ssl/SNIHostName.java line 104:
>
>> 102: * Users are advised to prefer the {@link #ofString(String)
>> ofString()}
>> 103: * static factory method over this constructor, since the former
>> performs
>> 104: * stricter checks.
>
> I don't know if this will be seen, which makes me wonder if this should go a
> step further and deprecate the constructors.
I'm glad you said this, I completely agree with your concern and reasoning. I
will do it.
Though this reminded me another question I need some help with: Shall I migrate
existing usages of `SNIHostName::new` to the new static factory methods in this
PR? Slim PRs are ideal, but leaving the correction of call-sites to a 2nd PR
bears the risk that we might end up having a 3rd PR if the 1st (this) one has
any problems.
> src/java.base/share/classes/javax/net/ssl/SNIHostName.java line 182:
>
>> 180: * RFC 6066: Transport Layer Security (TLS) Extensions: Extension
>> Definitions
>> 181: */
>> 182: public static SNIHostName ofString(String hostname) {
>
> ofHostName(String) and ofEncoded(byte) is another choice that would make it
> clear at the use sites.
I had considered how it would look like at the call-site:
SNIHostName.ofHostName(hostName); // Too verbose/noisy with repetition?
and then thought of:
SNIHostName.ofString(hostName);
SNIHostName.ofBytes(hostName);
But I will leave the decision on naming to those more experienced in this field.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30747#discussion_r3119943971
PR Review Comment: https://git.openjdk.org/jdk/pull/30747#discussion_r3119966987
