John A. Tamplin has posted comments on this change.
Change subject: Add NavigableSet, NavigableMap to GWT and retrofit TreeMap
and TreeSet to implement it.
......................................................................
Patch Set 2: Code-Review-1
(10 comments)
Generally good, but a number of nits and the big one is we need a bunch of
tests to cover the new functionality (and a couple of bits of changed
functionality -- if the new way is correct, we should have had failing
tests, so we need to add them).
....................................................
File user/super/com/google/gwt/emul/java/util/AbstractNavigableMap.java
Line 400: abstract Iterator<Entry<K, V>> descendingEntryIterator();
Javadoc for what implementing classes are expected to provide for these
abstract methods.
....................................................
File user/super/com/google/gwt/emul/java/util/NavigableMap.java
Line 2: * Copyright 2012 Google Inc.
2013
Line 25: Map.Entry<K, V> ceilingEntry(K key);
Javadoc for methods
....................................................
File user/super/com/google/gwt/emul/java/util/NavigableSet.java
Line 2: * Copyright 2012 Google Inc.
2013
Line 24: E ceiling(E e);
Javadoc for methods
....................................................
File user/super/com/google/gwt/emul/java/util/TreeMap.java
Line 596
Why delete this line?
Line 84: private boolean inRange(SubMapType type, K key,
The line break seems weird here and elsewhere -- is this formatted with the
standard GWT formatter?
Line 194: if (toInclusive ? (compare > 0) : (compare >= 0)) {
Maybe compare > 0 || (toInclusive && compare == 0) would save a comparison
in most cases?
Likewise below.
Line 487: return null;
Is null the proper return here? Previously we were throwing NSE. Either
way, there should probably be a test that verifies the correct result.
Line 545: return null;
Likewise here.
--
To view, visit https://gwt-review.googlesource.com/3650
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2800b2b54b06a7ef255e01ad44b11909d878362a
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Thomas Broyer <[email protected]>
Gerrit-Reviewer: Brian Slesinsky <[email protected]>
Gerrit-Reviewer: John A. Tamplin <[email protected]>
Gerrit-Reviewer: Leeroy Jenkins <[email protected]>
Gerrit-Reviewer: Thomas Broyer <[email protected]>
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
