dev@glassfish.java.net

Re: Suggestions for improvements: AMX (generics)

From: Lloyd L Chambers <Lloyd.Chambers_at_Sun.COM>
Date: Thu, 20 Jul 2006 16:39:15 -0700

2. The way AMX uses Generics is just horrible. It's not uncommon to
see signatures like:

The way most methods are defined, it is possible to write code that
doesn't require casts. The alternative requires casts for every
call. In some cases, compatibility prevented use of improved method
signatures including a "Class<T>: parameter.

Using JNDIResource as a generic example, one can write the following:

Set<JNDIResource> s = server.getResourceSet();
TypeCast.checkSet( s, JNDIResource.class ); // optional, throws
ClassCastException if elements are not all JNDIResource
for( JNDIResource jndi : s ) {
}

The alternatives according to the suggestion below is:

Set<J2EEResource> s = server.getResourceSet();
for( J2EEResource j : s ) {
        JNDIResource jndi = (JNDIResource)j; // explicit cast,
ClassCastException if not a JNDIResource
}

or:

Set<J2EEResource> s = server.getResourceSet();
Set<JNDIResource sj = TypeCast.checkSet( s, JNDIResource.class );
for( JNDIResource jndi : sj ) {
}

In this particular example, a API return type of Set<J2EEResource>
might be a better choice, given the potentially mixed variety of
subtypes that might be returned. There are other examples where the
type is known or implied by an argument.

I completely agree that getResourceSet( Class<T> type) is far better,
but that would have been an incompatible change (which we wished to
avoid), or an additional method. And getContaineeMap() already
provides such functionality, though for compatibility reasons the
type passed is String and not Class<T>.


>
> <T extends J2EEResource> Set<T> getResourcesSet();
>
> (this one is in J2EEServer), which is probably supposed to be
> either:
>
> Set<J2EEResource> getResourceSet()
>
> or:
>
> <T extends J2EEResource> Set<T> getResourceSet(Class<T> type);
>
> Another example is:
>
> <T1 extends Serializable,T2 extends Serializable>
> void startDeploy(Object deployID,
> Map<String,T1> source,
> Map<String,T2> plan,
> Map<String,String> options)
>
> ... which should have been just:
>
> void startDeploy(Object deployID,
> Map<String,? extends Serializable> source,
> Map<String,? extends Serializable> plan,
> Map<String,String> options)
>

I believe you are correct that the "? extends Serializable" choice
would be preferable in this case. The T1/T2 construct enforces
uniformity of type (though plain 'Serializable' would suffice also).