Lior Vernia has posted comments on this change. Change subject: frontend: refactoring: Generify list models ......................................................................
Patch Set 19: (3 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 { > Yes, this interface is used when we need to polymorphically treat instances It wasn't really polymorphic, was it? Is there any EntityModel passed to them? When you changed all these references from EntityModel to HasEntity, couldn't you change them to SearchableListModel or ListWithDetailsModel instead? 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> { > Yes, it is true that SortedListModel has nothing to with entities but its s "Its subclasses" is SearchableListModel. And the only other usage for ListModelWithEntity currently is by NetworkItemModel, where it seems it's not needed. So again, why not implement this functionality in SearchableListModel (which would implement HasEntity as needed) and lose the ListModelWithEntity class to begin with? Instead of deferring to "future refactoring"? Line 16: Line 17: static class SortSensitiveComparator<T> implements Comparator<T> { Line 18: Line 19: private final Comparator<? super T> comparator; http://gerrit.ovirt.org/#/c/32907/19/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkItemModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkItemModel.java: Line 9: * Line 10: * @param <T> Line 11: * The Item status enum type Line 12: */ Line 13: public abstract class NetworkItemModel<E, T extends Enum<T>> extends ListModelWithEntity<E, LogicalNetworkModel> implements Comparable<NetworkItemModel<E, T>> { > No, it just extends ListModelWithEntity which just adds entity E to plain L Sorry, my bad. Line 14: Line 15: private final HostSetupNetworksModel setupModel; Line 16: Line 17: private String error = null; -- 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
