Lior Vernia has posted comments on this change.

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


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/28392/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SortedCollection.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SortedCollection.java:

Line 38:             Collection<? extends E> initialValues) {
Line 39:         this.items = new TreeSet<E>(comparator);
Line 40: 
Line 41:         initComparator(comparator);
Line 42:         initPositions(initialValues);
Actually, by initiating the positions Map each time you create a new 
Collection, I think stability across refreshes because the state of ordering 
isn't preserved across calls to SortedListModel.sortItems().

For this to be preserved, I think the positions map has to not be part of the 
collection itself, as it needs to persist its state across creations of new 
collections.

In my original implementation, note that the positions map was NOT initialized 
each time sortItems() was called.

This same problem exists in the other alternative as well.
Line 43:         addAll(initialValues);
Line 44:     }
Line 45: 
Line 46:     SortedCollection(ComparatorWithPositionCallback<E> comparator) {


-- 
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: 1
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