mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From elijah baley <e_ba...@outlook.com>
Subject Re: mina-sshd git commit: [SSHD-805] Return a clone of the RFC8268 modulu number bytes to avoid inadvertent modification of the shared instance
Date Mon, 05 Mar 2018 17:10:01 GMT
> +    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

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message