Lior Vernia has posted comments on this change.

Change subject: webadmin,userportal: Fix client-side sort (ArrayList approach)
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/28392/2/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 84:         if (items == null || comparator == null) {
Line 85:             return items;
Line 86:         }
Line 87: 
Line 88:         List<T> sortedList = new ArrayList<T>() {
> The contract of List isn't compatible with what SortedListModel is supposed
Also, similarly to the comment I added in the other patchset, this data 
structure can't maintain stable ordering across refreshes as it doesn't refer 
to the previous collection of items each time new items are passed.

As in the other patchset, you need to "remember" the previous ordering somehow 
outside the data structure (because this memory needs to be persisted across 
creations of new sorted collections).
Line 89: 
Line 90:             private static final long serialVersionUID = 
499355412389856325L;
Line 91: 
Line 92:             @Override


-- 
To view, visit http://gerrit.ovirt.org/28392
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I437e9dd7ff2afdb2d87fb0ba6c17dcb081cd965d
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to