ADBA Add type safe newFactory method

[Douglas Surber]

I don’t have a strong argument one way or the other. Both String and Class seem fine. The most recent push takes a String but I have no objection to changing it if there is a good reason. I’m in no position to judge how significant an "elegant Kotlin reified generic type extension function” would be. 

Douglas

> On Jul 18, 2018, at 2:57 PM, Frédéric Montariol <frederic.montariol at gmail.com> wrote:
> 
> I really like the alternative newFactory static method taking a Class parameter :
> public static <T extends DataSourceFactory> T newFactory(Class<T> clazz) 
> It would allow elegant Kotlin reified generic type extension function !
> 
> But there is something I don't like, this is the fact that DataSourceFactory.newFactory returns null if no corresponding class is found, causing NPE on next method call : factory.builder()
> I would prefer if it throws an Exception.
> 
> Le ven. 13 juil. 2018 à 19:48, Douglas Surber <douglas.surber at oracle.com <mailto:douglas.surber at oracle.com>> a écrit :
> The internal consensus is to change the spec exactly as you suggest and require the explicit cast if that’s what the user wants. I still would like to hear what others think.
> 
>   public static DataSourceFactory newFactory(String name) { … }
> 
>   DataSourceFactory factory = DataSourceFactory.newFactory(“my.adba.MyFactory”);
> 
> or 
> 
>   MyFactory factory = (MyFactory) DataSourceFactory.newFactory(“my.adba.MyFactory”);
> 
> Douglas
> 
> 
> 
> > On Jul 13, 2018, at 9:46 AM, Douglas Surber <douglas.surber at oracle.com <mailto:douglas.surber at oracle.com>> wrote:
> > 
> > This is a good question. I defined it the way I did specifically to avoid the explicit cast. I have no strong opinion on which is better. I’ll ask for opinions internally and I hope other members of this list give their thoughts.
> > 
> > Douglas
> > 
> >> On Jul 13, 2018, at 8:26 AM, Mark Rotteveel <mark at lawinegevaar.nl <mailto:mark at lawinegevaar.nl>> wrote:
> >> 
> >> The existing DataSourceFactory.newFactory(String name) has an unchecked cast using a (possibly inferred) type parameter.
> >> 
> >> Would it make sense to add an alternative with the signature:
> >> 
> >> public static <T extends DataSourceFactory> T newFactory(Class<T> clazz)
> >> 
> >> When you use this, you are tied to an ADBA implementation at compile time, but it will make this more type-safe.
> >> 
> >> And maybe the unchecked cast (and type parameter) should be removed from the variant taking a name only.
> >> 
> >> That is, change the method to
> >> 
> >> public static DataSourceFactory newFactory(String name) {
> >>   if (name == null) throw new IllegalArgumentException("DataSourceFactory name is null");
> >>   return ServiceLoader
> >>           .load(DataSourceFactory.class)
> >>           .stream()
> >>           .filter(p -> p.type().getName().equals(name))
> >>           .findFirst()
> >>           .map(Provider::get)
> >>           .orElse(null);
> >> }
> >> 
> >> Any casts would then be the explicit responsibility of the developer, instead of accidental due to type inference.
> >> 
> >> Mark
> >> -- 
> >> Mark Rotteveel
> > 
>