On 30 June 2016 at 11:01, Stian Soiland-Reyes <st...@apache.org> wrote:
> On 30 June 2016 at 00:53, sebb <seb...@gmail.com> wrote:
>> As the subject says; the two factories use a different naming convention.
>>
>> Would it be sensible to standardise on getInstance, given that the
>> class name says what the instance will be?
>
> Hm, but CryptoRandomFactory.getCryptoRandom() returns a CryptoRandom,
> and CryptoCipherFactory.getInstance() returns a CryptoCipher() (not a
> CryptoCipherFactory).
>
> I think it would be misleading to call it getInstance() as that
> implies the singleton pattern, e.g. to return the *Factory, but in
> fact neither of the factories can be instantiated (private
> constructor, only static methods).

getInstance() does not imply Singleton to me, for example:

Calendar.getInstance()
TimeZone.getInstance()

> Both methods take parameters, so it would be returning different
> instances depending on the parameters - so although the ciphers etc.
> might always be the same instances, it's not the singleton pattern.
> (But then not quite the factory pattern either :) )
>
>
> Agreed that the two factories should however be consistent, so I would 
> suggest:
>
>
> public class CryptoRandomFactory {
>   public static CryptoRandom getCryptoRandom( .. ) { .. }
> }
>
> public class CryptoCipherFactory {
>   public static CryptoCipher getCryptoCipher( ..)  { .. }
> }

That actually results in the fewest changes to the source currently.

It would also allow the following usage:

import static 
org.apache.commons.crypto.random.CryptoRandomFactory.getCryptoRandom;
...
CryptoRandom random = getCryptoRandom(properties);

>
> But I understand your argument.. because for the caller it will be
> quite repetitive:
>
> CryptoCipher cryptoCipher = CryptoCipherFactory.getCryptoCipher( .. ) ;
> CryptoRandom cryptoRandom = CryptoRandomFactory.getCryptoRandom( .. ) ;
>
>
> Perhaps just call the methods  just  .get(...) instead?
>
>
> public class CryptoRandomFactory {
>   public static CryptoRandom get( .. ) { .. }
> }
>
> public class CryptoCipherFactory {
>   public static CryptoCipher get( ..)  { .. }
> }
>

Cannot then use static import of both.

>
> --
> Stian Soiland-Reyes
> Apache Taverna (incubating), Apache Commons
> http://orcid.org/0000-0001-9842-9718
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to