Hi,

this is the relevant discussion why I reverted the ListView constructor to that of Wicket 6:

http://mail-archives.apache.org/mod_mbox/wicket-dev/201409.mbox/%3CCAJmbs8gDa5mJgwbkoOZS3oH5TYZZ-Ap3_SFDjBHs5SYpn4zTkg%40mail.gmail.com%3E

Changing it would mean an API break for existing applications. I don't mind

ListView exposes a read-write view on the contents of the list, via 
ListView.getModelObject(),
but also ListItem.setModelObject

I wanted ListItem to be read-only, but Martin an I agreed on keeping it 
writeable for backwards-compatibility.
Is this really a problem we have to fix in our API?

Regards
Sven


On 22.06.2015 14:31, Emond Papegaaij wrote:
Hi Sven,

It's easy to change the testcase to:
     class NumberListView<N extends Number> extends ListView<N>
and
     new NumberListView<Integer>("integers", integers)

I think the constructor in Wicket 6 is wrong. It should be IModel<? extends
List<T>> model: we don't care what type the list is, but we do care about the
type of the contents of the list. ListView exposes a read-write view on the
contents of the list, via ListView.getModelObject(), but also
ListItem.setModelObject. IMHO it should either be read-only with ? extends T
or read-write with T, but mixed. The first is a major API break, the second is
not, so I prefer the second.

Best regards,
Emond

On Monday 22 June 2015 14:22:25 Sven Meier wrote:
Hi Martijn,

the ListView constructor is now as it has been in Wicket 6:

      public ListView(final String id, final IModel<? extends List<?
extends T>> model)

ListViewTest#generics() shows a valid use case, that is not possible
otherwise.

Regards
Sven

On 22.06.2015 13:42, Martijn Dashorst wrote:
I'm not sure I'm fan of this change. Using these wildcards breaks all
kinds of code. What is the benefit?

The way it is implemented currently is also inconsistent: ListItem is
typed as ListItem<T> but it should be ListItem<? extends T>. This
gist: https://gist.github.com/dashorst/4ee7ab1696321f290a24 shows how
this should be implemented.

HOWEVER: I don't actually propose such a change, but rather have
adcb7a632af8225e86e09e398b8fb5430b143b18 be reverted. The linked patch
will break the world and for little to no benefit.
adcb7a632af8225e86e09e398b8fb5430b143b18 breaks API in a couple of
places, but I don't see the benefit of those breaks as well.

Martijn

Reply via email to