The trouble with Generics

This document tries to capture the benefits and doubts we have regarding generics in Wicket 1.4. Ultimately we need to find a way where the Framework and Wicket Community can move onward to Wicket 1.5/2.0 with Java 5 as a basis.

Generics Rock!

Java generics are good for type safety throughout your code. They document the code directly with intent: List<Person> leaves little to the imagination as to what goes into the list.

These benefits are a great asset to Wicket. Wicket uses the concept of models to provide components with the data that needs to be rendered. Until recently Wicket was Java 1.4 based, and a lot of components suffered from the lack of generified models. Take for instance the ListView:

Generics Suck!

TBD

Generics are warranted for IModel but too much verbosity for Component

If there is any place in Wicket which clearly holds typed data, it is the models (the IModel interface). In practice, one ends up creating a lot of custom components without models, such as different subclasses of MarkupContainer and possibly Page, and here the Component generification just adds more verbosity to the code.

Typically when starting to use a custom component, you start by typing its constructor call, so the constructor of a custom component is a crucial place for making clean and easy APIs. The possibility of giving type arguments to the IModel instance being passed in there is great, DropDownChoice is a prime example as it also has constructors with a List that needs to contain objects of the same type as the model. However,

DropDownChoice fruit = new DropDownChoice("fruit", new Model<Fruit>(), Arrays.asList(apple, orange, banana));

is better than

DropDownChoice<Fruit> fruit = new DropDownChoice<Fruit>("fruit", new Model<Fruit>(), Arrays.asList(apple, orange, banana));

Also when using 2.0 the experience was that IModel type was excellent, but Component type was more like noise. When using 1.3, the lack of types has hardly caused trouble; mostly types would be helpful when first creating a new component. Once you have the component constructed and are calling getModelObject() on it, you're typically sure of what to expect. There have not been difficult problems concerning objects of wrong type inside models.

In this approach, using getModelObject() you cast the same as in 1.3, and for getModel() you can do the necessary casting cleanly, which leaves you in a situation better than in 1.3.
(Timo Rantalaiho)

I mostly agree with Timo. The only thing we lose from Component is generification of IConverter. Since IConverter is mostly only important in FormComponent perhaps we can fully type FormComponent which would allow us to have typesafe converters AND validators.
(Igor Vaynberg)

If we dont do Component but we do specific subtypes like FormComponent that the above DropDownChoice example still has that signature..
and then i also vote for Repeaters to be generified, because especially in the onPopulate it is very descriptive what you get..
But i think people will then start complaining because they are using Links a lot and there model object.. So then we do get the question constantly on the list Why is this and Why isnt that... (i think)
I still dont find this that bad.. It really describes it:

DropDownChoice<Fruit> fruit = new DropDownChoice<Fruit>("fruit", new Model<Fruit>(), Arrays.asList(apple, orange, banana));

because what you also could do is this:

DropDownChoice<Fruit> fruit = new DropDownChoice<Fruit>("fruit", Arrays.asList(apple, orange, banana));

and yes i know.. this is somewhere not really type safe..because now a CompoundModel is used and that is based on strings..
But in the code is more descriptive.. but i guess somehow we should have a warning
And this is also the reason why i am also working on type safe property models.
(Johan Compagner)

Suggestion 1 — }}{{{}setResponsePage(){} --signature

The initial issue that started this discussion was a problem with this method signature:

public final void setResponsePage(final Class<? extends Page<?>> cls, PageParameters parameters)
{
    getRequestCycle().setResponsePage(cls, parameters);
}

We could change this to:

@SuppressWarnings({"unchecked"})
public final void setResponsePage(final Class<?> cls, PageParameters parameters)
{
    final Class<? extends Page<?>> castCls = (Class<? extends Page<?>>) cls;
    getRequestCycle().setResponsePage(castCls, parameters);
}

This way (a) users migrating to 1.4 don't get tons of warnings/errors and (b) if they pass in the wrong thing, a class cast exception will be thrown quickly.

NOTE: This is just an example - we may wish to move the cast into the getRequestCycle() call ...

My idea is to dial back the generics on some of the API calls like this one and others that cause pain, but leave the generics for useful things like getModelObject().

-Doug Donohoe

This is already resolved with the following signature:

public final <C extends Page<?>> void setResponsePage(final Class<C> cls, PageParameters parameters)
{
    getRequestCycle().setResponsePage(cls, parameters);
}

Gerolf Seitz

Suggestion 2 - RestartResponseAtInterceptPageException constructor

I agree with Doug, and with suggestion 1 above on setResponsePage changing to take Class<?>

I think another example is the constructor of RestartResponseAtInterceptPageException (and similar classes) that take a class as an argument.

public RestartResponseAtInterceptPageException(final Class< ? extends Page< ? >> interceptPageClass)
	{
		if (interceptPageClass == null)
		{
			throw new IllegalStateException("Argument pageClass cannot be null");
		}
		redirectToInterceptPage(interceptPageClass);
	}

We could change this to:

public RestartResponseAtInterceptPageException(final Class<?> interceptPageClass)
{
	if (interceptPageClass == null || !Page.class.isAssignableFrom(interceptPageClass))
	{
		throw new IllegalStateException("Argument pageClass cannot be null and must inherit from Page.class");
	}
	redirectToInterceptPage(interceptPageClass);
}

This, for the same reason as suggestion 1 - it's not very likely that someone is going to pass a non-page-child class into these methods or constructors, so the benefit is minimal, but the strictly generified versions penalize you if your page class is not also generified, which would be confusing to new users.

Same as above. -Gerolf Seitz

Generics "Gotchas"

Palette

The current (1.4-m2) Palette class' constructor signature looks like this:

public Palette(String id, 
               IModel<Collection<T>> model,
	       IModel<Collection<? extends T>> choicesModel, 
               IChoiceRenderer<T> choiceRenderer, 
               int rows,
	       boolean allowOrder)

This doesn't allow me to use an IModel<List<T>>. How about if we change it to:

public Palette(String id, 
               IModel<? extends Collection<T>> model,
	       IModel<? extends Collection<? extends T>> choicesModel, 
               IChoiceRenderer<T> choiceRenderer, 
               int rows,
	       boolean allowOrder)

This appears to make things nicer in my local if I change it.