On Thu, 9 Feb 2023 13:17:30 GMT, Maurizio Cimadamore <[email protected]>
wrote:
>> Adam Sotona has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> AttributeElement.Kind removal (#48)
>
> src/java.base/share/classes/jdk/internal/classfile/Classfile.java line 66:
>
>> 64: * @param <V> the type of the optional value
>> 65: */
>> 66: public sealed interface Option<V> permits Options.OptionValue {
>
> After looking more at the usages of Options it is not clear to me as to why
> generics are needed. Lookup is always performed using a non-generic
> Option.Key - so the API code has to be unchecked anyway. In fact, I'm not
> even sure you need a `value()` method. When looking at usages, Option is
> mostly something you create when you need to pass it to something else (e.g.
> a constant pool, etc.). Since the client has created the option, it is not
> clear to me that the client has a need to access the option value (which the
> API implementation can access internally by other means).
You are right, I've tried to remove generics from `Option` and `Option::value`
method and it didn't have any impact on any use case we have.
All access to the `Option` value is done through `ClassReader::optionValue` or
`ConstantPoolBuilder::optionValue`, there was no use of `Option::value` at all.
I think it is valuable API cleanup, thanks!
-------------
PR: https://git.openjdk.org/jdk/pull/10982
