Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom : CryptoCipherFactory.getInstance

2016-06-30 Thread Matt Sicker
If you were using Java 8, you could totally have the factory in the
interface now.

On 30 June 2016 at 10:12, sebb  wrote:

> On 30 June 2016 at 13:59, Emmanuel Bourg  wrote:
> > Would it make sense to move the factory method to the base class?
> >
> >CryptoRandom random = CryptoRandom.getInstance();
>
> It's an interface, not a base class.
>
> > Emmanuel Bourg
> >
> >
> > -
> > 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
>
>


-- 
Matt Sicker 


Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom : CryptoCipherFactory.getInstance

2016-06-30 Thread sebb
On 30 June 2016 at 13:59, Emmanuel Bourg  wrote:
> Would it make sense to move the factory method to the base class?
>
>CryptoRandom random = CryptoRandom.getInstance();

It's an interface, not a base class.

> Emmanuel Bourg
>
>
> -
> 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



Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom : CryptoCipherFactory.getInstance

2016-06-30 Thread Stian Soiland-Reyes
On 30 June 2016 at 12:15, sebb  wrote:

> It would also allow the following usage:
> import static 
> org.apache.commons.crypto.random.CryptoRandomFactory.getCryptoRandom;
> CryptoRandom random = getCryptoRandom(properties);

I didn't consider static import. You've won me over,  the above is best! :)


-- 
Stian Soiland-Reyes
Apache Taverna (incubating), Apache Commons
http://orcid.org/-0001-9842-9718

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



Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom : CryptoCipherFactory.getInstance

2016-06-30 Thread Emmanuel Bourg
Would it make sense to move the factory method to the base class?

   CryptoRandom random = CryptoRandom.getInstance();

Emmanuel Bourg


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



Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom : CryptoCipherFactory.getInstance

2016-06-30 Thread sebb
On 30 June 2016 at 12:37, Benedikt Ritter  wrote:
> sebb  schrieb am Do., 30. Juni 2016 um 13:16 Uhr:
>
>> On 30 June 2016 at 11:01, Stian Soiland-Reyes  wrote:
>> > On 30 June 2016 at 00:53, sebb  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()
>>
>
> Those APIs are just bad IMHO. We shouldn't use them as an example :-)
>

Furthermore, an instance is definitely not a singleton.

If it is true that Factory.getInstance implies singleton then that is
bad also ...

One of the reasons for preferring a static factory method over a
constructor is that it allows the code to return either a singleton
instance or a new instance as appropriate.

Particularly in the case of a method with a parameter, it should be
clear that the returned object is not a singleton.

Singletons only make sense if there is only one possible instance, but
any parameter can take multiple values and must affect the object -
otherwise the parameter is useless.

However this is all moot, as I have made the names consistent.

>>
>> > 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/-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
>>
>>

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



Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom : CryptoCipherFactory.getInstance

2016-06-30 Thread Benedikt Ritter
sebb  schrieb am Do., 30. Juni 2016 um 13:16 Uhr:

> On 30 June 2016 at 11:01, Stian Soiland-Reyes  wrote:
> > On 30 June 2016 at 00:53, sebb  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()
>

Those APIs are just bad IMHO. We shouldn't use them as an example :-)


>
> > 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/-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
>
>


Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom : CryptoCipherFactory.getInstance

2016-06-30 Thread sebb
On 30 June 2016 at 11:01, Stian Soiland-Reyes  wrote:
> On 30 June 2016 at 00:53, sebb  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/-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



Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom : CryptoCipherFactory.getInstance

2016-06-30 Thread Stian Soiland-Reyes
On 30 June 2016 at 00:53, sebb  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).

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( ..)  { .. }
}


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( ..)  { .. }
}



-- 
Stian Soiland-Reyes
Apache Taverna (incubating), Apache Commons
http://orcid.org/-0001-9842-9718

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



Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom : CryptoCipherFactory.getInstance

2016-06-29 Thread Gary Gregory
On Wed, Jun 29, 2016 at 4:53 PM, sebb  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?
>

+1

Gary


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


-- 
E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
Java Persistence with Hibernate, Second Edition

JUnit in Action, Second Edition 
Spring Batch in Action 
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory


[CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom : CryptoCipherFactory.getInstance

2016-06-29 Thread sebb
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?

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