On Fri, 1 Nov 2024 23:57:20 GMT, Chen Liang <li...@openjdk.org> wrote:
>> src/java.base/share/classes/jdk/internal/constant/ConstantUtils.java line
>> 258:
>>
>>> 256: if (name.isEmpty())
>>> 257: return name;
>>> 258: return validateBinaryClassName(name);
>>
>> Perhaps have a utility method to specify if empty name is allowed.
>> `validateBinaryClassName` and `validateInternalClassName` only differ in the
>> separator charactor. They can be refactored to call one single name
>> validation method something like this:
>>
>>
>> private static String validateName(String name, boolean internalName,
>> boolean allowEmptyName) {
>> if (!allowEmptyName && name.isEmpty())
>> throw invalidClassName(name);
>>
>> // state variable for detection of illegal states, such as:
>> // empty unqualified name, consecutive, leading, or trailing
>> separators
>> int afterSeparator = 0;
>> int len = name.length();
>> for (int i = 0; i < len; i++) {
>> char ch = name.charAt(i);
>> // reject ';' or '[' or other form's separator
>> if (ch == ';' || ch == '[' || (internalName && ch == '.') ||
>> (!internalName && ch == '/'))
>> throw invalidClassName(name);
>> if ((internalName && ch == '/') || (!internalName && ch == '.'))
>> {
>> // illegal state when received separator indicates
>> consecutive
>> // or leading separators
>> if (i == afterSeparator)
>> throw invalidClassName(name);
>> afterSeparator = i + 1;
>> }
>> }
>> // reject empty unqualified name or trailing separators
>> if (len == afterSeparator)
>> throw invalidClassName(name);
>> return name;
>> }
>
> I assume I will still keep the 4 public methods to call this implementation
> method, as the boolean arguments will be otherwise confusing. Is that what
> you intend?
yes. since you already made some refactoring, can improve further.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21830#discussion_r1826395001
