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

Reply via email to