On 24 Jul 2015, at 20:30, Martin Buchholz <marti...@google.com> wrote:
> Hi Steve, pleased to meet you! > > (are you a new maintainer of java.util collections?) > I suggested that Steve look at some “simple” issues to build up credits for a committer role, so i threw this and some others over the fence for Steve to review and possibly fix. > I'm mildly opposed to this change - I added comments to the bug. > It looks like you are not the only one. I am outnumbered :-) My guiding principle here was that argument validation should not result in side-effects. Thus the state of a collection should remain unchanged if an exception is thrown due to an invalid argument of an operation. ArrayList.remove is inconsistent with regards to nearly all the other ArrayList methods, add(int, E) and addAll(int, Collection) for an out of bounds index, and addAll(Collection ), removeAll, retainAll, removeIf, replaceAll and sort for a null argument. It’s not inconsistent with ArrayList.removeRange, except for if an invalid range from > t, but is inconsistent with AbstractList.removeRange. It’s also inconsistent LinkedList and CopyOnWriteArrayList *and* sub-lists of ArrayList and AbstractList. And inconsistent more generally for other Collection implementations, such as Queue.add/offer/remove implementations that do not accept null values. The behaviour of ArrayList.remove, and also ArrayList.removeRange, are outliers [*]. It’s also a rather obscure difference behaviour with likely minimal impact if changed (far less so than the change to Arrays.toList) . Paul. [*] While there is a argument to be had for updating concurrent modification state of a collection for an operation that performs structural modification regardless of whether its arguments are valid or not such a behavioural change would have far more impact. > On Fri, Jul 24, 2015 at 11:16 AM, Steve Drach <steve.dr...@oracle.com> > wrote: > >> Hello, >> >> Please review the fix for JDK-8114832. I did what was suggested in the >> comments in the bug report, moving the increment of modcount to the “right” >> place. >> >> https://bugs.openjdk.java.net/browse/JDK-8114832 < >> https://bugs.openjdk.java.net/browse/JDK-8114832> >> http://slc01hfj.us.oracle.com/webrevs/JDK-8114832/webrev/index.html < >> http://slc01hfj.us.oracle.com/webrevs/JDK-8114832/webrev/index.html> >> >> An internal review has been completed. >> >> Thanks, >> Steve >> >> >>
