Hi Stephen,
The aspect of the current implementation that is problematic is the
copying of the set,
its not just single object creation but an entry for every ZoneID.
Adding a size method exposing some internal state increases the
possibility that
when it is used independently it will be out of sync. Not a big issue,
but one to avoid
when designing an API.
Exposing a mutable Hashset was probably a mistake but one that cannot be
corrected
now. The optics aren't concerning to me.
SharedSecrets are much messier and not appropriate.
Adding a method to provide what was originally needed is a cleaner solution.
Roger
On 5/5/2016 11:27 AM, Stephen Colebourne wrote:
I fail to see why adding a new read-only method alongside the existing
method adds any more value to the API than adding a new size method.
At least with the size method the API is still sensible - a mutable
and immutable method alongside each other shouts out that a mistake
was made. A size method is more subtle about the mistake (plenty of
APIs have a size method alongside a collection method).
Finally, a read-only method involves object creation, thus has worse
performance than adding a size method.
The only other alternative is perhaps to use SharedSecrets? I don't
know what possibilities there are there. If not SharedSecrets, then no
matter what, we are adding "a trivial method to the public API used
only for an optimization".
Stephen
On 5 May 2016 at 15:23, Roger Riggs <roger.ri...@oracle.com> wrote:
Hi Bhanu,
Adding a trivial method to the public API used only for an optimization is
not a good fix for this issue.
A better fix was suggested to add a non-copying read-only version of
ZoneRulesProvider.getAvailableZoneIds()
Please revise the fix to instead implement and use:
/**
* Gets a readonly set of available zone IDs.
* <p>
* These IDs are the string form of a {@link ZoneId}.
*
* @return a unmodifiable copy of the set of zone IDs, not null
*/
public static Set<String> getReadOnlyAvailableZoneIds() {
return Collections.unmodifiableSet(ZONES.keySet());
}
Roger
On 5/5/2016 5:10 AM, Bhanu Gopularam wrote:
Hi all,
Please review fix following issue
Bug Id : https://bugs.openjdk.java.net/browse/JDK-8066291
Solution: provided new method to get size of available zone ids
webrev : http://cr.openjdk.java.net/~bgopularam/bhanu/JDK-8066291/webrev.00
Thanks,
Bhanu
