Alexander Wels has posted comments on this change.

Change subject: webadmin: Render client-side sorting more robust
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.ovirt.org/#/c/28268/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 58: 
Line 59:     }
Line 60: 
Line 61:     protected Comparator<? super T> comparator;
Line 62:     private final Map<T, Integer> positions = new HashMap<T, 
Integer>();
Why are we using a map with integer indexes? Wouldn't a regular list work just 
the same? Since the list would have ordering already?
Line 63: 
Line 64:     public SortedListModel(Comparator<? super T> comparator) {
Line 65:         super();
Line 66:         setComparator(comparator);


Line 88:         if (items == null || comparator == null) {
Line 89:             return items;
Line 90:         }
Line 91: 
Line 92:         initPositions(items);
So how is doing getItems().add(item) going to work in this scenario? it 
obviously won't be added to the positions map.
Line 93:         SortedSet<T> sortedItems = new TreeSet<T>(comparator);
Line 94:         sortedItems.addAll(items);
Line 95:         return sortedItems;
Line 96:     }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffd1ce7763e516ad109fdae5d584b183ee44117c
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Daniel Erez <[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