You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sebb <se...@gmail.com> on 2016/06/29 23:53:15 UTC
[CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom : CryptoCipherFactory.getInstance
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-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom
: CryptoCipherFactory.getInstance
Posted by Gary Gregory <ga...@gmail.com>.
On Wed, Jun 29, 2016 at 4:53 PM, sebb <se...@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?
>
+1
Gary
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom
: CryptoCipherFactory.getInstance
Posted by sebb <se...@gmail.com>.
On 30 June 2016 at 12:37, Benedikt Ritter <br...@apache.org> wrote:
> sebb <se...@gmail.com> schrieb am Do., 30. Juni 2016 um 13:16 Uhr:
>
>> On 30 June 2016 at 11:01, Stian Soiland-Reyes <st...@apache.org> wrote:
>> > On 30 June 2016 at 00:53, sebb <se...@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()
>>
>
> 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/0000-0001-9842-9718
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > For additional commands, e-mail: dev-help@commons.apache.org
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom
: CryptoCipherFactory.getInstance
Posted by Benedikt Ritter <br...@apache.org>.
sebb <se...@gmail.com> schrieb am Do., 30. Juni 2016 um 13:16 Uhr:
> On 30 June 2016 at 11:01, Stian Soiland-Reyes <st...@apache.org> wrote:
> > On 30 June 2016 at 00:53, sebb <se...@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()
>
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/0000-0001-9842-9718
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom
: CryptoCipherFactory.getInstance
Posted by Stian Soiland-Reyes <st...@apache.org>.
On 30 June 2016 at 12:15, sebb <se...@gmail.com> 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/0000-0001-9842-9718
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom
: CryptoCipherFactory.getInstance
Posted by sebb <se...@gmail.com>.
On 30 June 2016 at 11:01, Stian Soiland-Reyes <st...@apache.org> wrote:
> On 30 June 2016 at 00:53, sebb <se...@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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom
: CryptoCipherFactory.getInstance
Posted by Matt Sicker <bo...@gmail.com>.
If you were using Java 8, you could totally have the factory in the
interface now.
On 30 June 2016 at 10:12, sebb <se...@gmail.com> wrote:
> On 30 June 2016 at 13:59, Emmanuel Bourg <eb...@apache.org> 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-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
Matt Sicker <bo...@gmail.com>
Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom
: CryptoCipherFactory.getInstance
Posted by sebb <se...@gmail.com>.
On 30 June 2016 at 13:59, Emmanuel Bourg <eb...@apache.org> 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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom
: CryptoCipherFactory.getInstance
Posted by Emmanuel Bourg <eb...@apache.org>.
Would it make sense to move the factory method to the base class?
CryptoRandom random = CryptoRandom.getInstance();
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [CRYPTO] inconsistent naming CryptoRandomFactory.getCryptoRandom
: CryptoCipherFactory.getInstance
Posted by Stian Soiland-Reyes <st...@apache.org>.
On 30 June 2016 at 00:53, sebb <se...@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).
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/0000-0001-9842-9718
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org