Vojtech Szocs has posted comments on this change.

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


Patch Set 2:

I've uploaded two different patchsets to demonstrate two approaches to fix 
client-side sort (items disappearing due to original use of TreeSet):

1, TreeSet approach
* custom SortedCollection implementation ensures that "items" and "positions" 
live in the same scope, this is important because we don't want "positions" to 
keep referencing individual items that are not needed anymore (preventing 
memory leaks)
* SortedCollection makes best attempt to keep "items" and "positions" in sync, 
we don't want "positions" to keep referencing obsolete items
* if positions are equal (edge case that shouldn't happen), 
SortSensitiveComparator falls back to hashCode comparison

2, ArrayList approach
* significantly simpler and less error-prone than TreeSet approach
* only add(element) + addAll(collection) sort the list, other methods such as 
add(index, element) do not on purpose -- if caller uses these, he is expressing 
his intention to override order of items on his own

SortedListModelTest is the same for both patchsets.

Conceptually, list models work with list-like collections, most typically 
ArrayList received from GWT RPC response. There is still some code that assumes 
list model's getItems method should return list, quoting Frantisek:

"
happens after (client) sorting of disks (disks subtab in vm main tab). after 
that the backing collection becomes treeset, but our code in edit() method 
requires arraylist.
reproducible for network interfaces subtab as well.
"

And yes, computational cost of sorting entire list on single add(element) is 
not very efficient, however it's important to note that this is NOT the typical 
use case for list models; list models usually receive their items via setItems 
method.

-- 
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: Lior Vernia <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: No
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to