Martin Betak has posted comments on this change. Change subject: frontend: refactoring: Generify list models ......................................................................
Patch Set 19: (2 comments) http://gerrit.ovirt.org/#/c/32907/19/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/HasEntity.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/HasEntity.java: Line 3: import org.ovirt.engine.ui.uicompat.Event; Line 4: import org.ovirt.engine.ui.uicompat.EventArgs; Line 5: import org.ovirt.engine.ui.uicompat.EventDefinition; Line 6: Line 7: public interface HasEntity<T> extends IModel { > It wasn't really polymorphic, was it? Is there any EntityModel passed to th Indeed there are use cases when we intermingle EntityModel<E> with ListWithEntity<E, T> for example see VmListModel#setDetailList when we set a list of detail modes. Some models are simple such as vmGeneralModel (that ultimately extends EntityModel<VM>) other have sub list attached to the entity, such as VmDiskListModel, VmEventListmodel..... and these all implement HasEntity<VM> and base ListWithDetailsModels treats them exactly as such. HasEntity is the right interface for this case. Line 8: Line 9: EventDefinition entityChangedEventDefinition = new EventDefinition("EntityChanged", HasEntity.class); //$NON-NLS-1$; Line 10: Line 11: Event<EventArgs> getEntityChangedEvent(); http://gerrit.ovirt.org/#/c/32907/19/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModel.java: Line 11: /** Line 12: * @param <E> {@link org.ovirt.engine.ui.uicommonweb.models.ListModelWithEntity.E} Line 13: * @param <T> {@link org.ovirt.engine.ui.uicommonweb.models.ListModelWithEntity.T} Line 14: */ Line 15: public class SortedListModel<E, T> extends ListModelWithEntity<E, T> { > "Its subclasses" is SearchableListModel. And the only other usage for ListM Ok, now I see your point. In case we will not have other uses for LMWE (other than via subclassing SearchableListModel) we can get rid of this class. My original design was to separate concerns (from object-oriented standpoint) and responsibility of this class would be only adding the entity feature. I will try and see how it would look with LWME squashed into SearchableListModel. Line 16: Line 17: static class SortSensitiveComparator<T> implements Comparator<T> { Line 18: Line 19: private final Comparator<? super T> comparator; -- To view, visit http://gerrit.ovirt.org/32907 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I00f1671a5839fe31b49ad0d1c99bad84e5f0c7c4 Gerrit-PatchSet: 19 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Betak <mbe...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Jakub Niedermertl <jnied...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Martin Betak <mbe...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches