> + public static byte[] readOakleyGroup(String name) {
>
The method was private, but making it public would need just a bit of
javadoc explaining the input/output.
I don't really see the need, since we have the getP16() and similar methods
that are already public though...
>> In general in an open-source project we want to keep private methods to a
>> bare minimum (if at all) - my view is that ANYTHING
>> should be re-usable and/or overridable. I do take your point though about
>> adding documentation as to the input/output and throws
> + byte[] value = OAKLEY_GROUPS.computeIfAbsent(name, DHGroupData::
> doReadOakleyGroup);
> + return (value == null) ? null : value.clone();
>
value can't be null, so this check is unnecessary.
>> semantically true but in general 'computeIfAbsent' documentation states:
If the specified key is not already associated with a value (or is mapped to
null), attempts to compute its value using the given mapping function and
enters it into this map unless null.
>> so, just for coding safety I think this check is in order - although I agree
>> that WE know doReadOakleyGroup never returns a null
>
> - private static byte[] doReadOakleyGroup(String name) {
> + public static byte[] doReadOakleyGroup(String name) {
>
I don't think that one should be public given readOakleyGroup has been made
public already. If you really want to keep that one public, it should be
renamed as doXxx
are usually meant to be private methods.
>> agreed - will call it 'readOkelyGroupFromResource'
--
------------------------
Guillaume Nodet
