LGTM

I recommended a bit of extra javadoc, but code looks good.


http://gwt-code-reviews.appspot.com/1674803/diff/4001/user/src/com/google/gwt/view/client/OrderedMultiSelectionModel.java
File user/src/com/google/gwt/view/client/OrderedMultiSelectionModel.java
(right):

http://gwt-code-reviews.appspot.com/1674803/diff/4001/user/src/com/google/gwt/view/client/OrderedMultiSelectionModel.java#newcode26
user/src/com/google/gwt/view/client/OrderedMultiSelectionModel.java:26:
* retains order of selection.
Add this to the JavaDoc.  Normally I don't think its necessary to
include implementation details, but in this case its important to
understand the cost of using this class.

OrderedMultiSelectionModel uses LinkedHashMaps, which may increase the
size of your compiled output if you do not use LinkedHashMaps elsewhere
in your application.

http://gwt-code-reviews.appspot.com/1674803/diff/4001/user/src/com/google/gwt/view/client/OrderedMultiSelectionModel.java#newcode52
user/src/com/google/gwt/view/client/OrderedMultiSelectionModel.java:52:
* @return the list of selected items in the order of additions
Can you define how setting existing items is handled?  It looks like
setSelected calls Map.setSelected(), which I assume moves the selected
item to the end of the list.

http://gwt-code-reviews.appspot.com/1674803/diff/4001/user/test/com/google/gwt/view/client/OrderedMultiSelectionModelTest.java
File
user/test/com/google/gwt/view/client/OrderedMultiSelectionModelTest.java
(right):

http://gwt-code-reviews.appspot.com/1674803/diff/4001/user/test/com/google/gwt/view/client/OrderedMultiSelectionModelTest.java#newcode43
user/test/com/google/gwt/view/client/OrderedMultiSelectionModelTest.java:43:

Right here, select "test0" (which is already selected), and verify that
it moves to the end of the list.

http://gwt-code-reviews.appspot.com/1674803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to