On Fri, Dec 14, 2012 at 4:54 AM, Remi Forax <[email protected]> wrote: > Hi Akhil, > > On 12/14/2012 02:24 AM, Akhil Arora wrote: >> >> As part of the library lambdafication, this patch adds a forEach default >> method to Iterator, and converts remove() into a default method so that >> implementations of Iterator no longer have to override remove if they >> desire the default behavior, which is to throw an >> UnsupportedOperationException. >> >> http://cr.openjdk.java.net/~akhil/8005051.0/webrev/ >> >> The above patch requires a small patch to an internal class which >> happens to implement both Iterable and Iterator. Now both Iterable and >> Iterator supply a default forEach method, so the compiler balks. One >> minimally intrusive solution is for this class to override both defaults >> and provide its own version of forEach. >> >> http://cr.openjdk.java.net/~akhil/8005053.0/webrev/ > > > The issue here is in the code of ASM wich is an external library imported in > the JDK, > so not sure it's a good idea to patch it. > > Moreover, goggling for "implement Iterable, Iterator" shows that this > anti-pattern is used too frequently to not step back and see if a better > solution is not possible. > https://encrypted.google.com/search?hl=en&q=%22implements+Iterable%3C*%3E%2C+Iterator%3C*%3E%22 > > We can't remove Collection.forEach without having perf issue because the > stream pipeline use it. > Iterator.forEach can be removed but it's a pity because it's really > convenient, > a possble solution is to rename Iterator.forEach() to something else.
I agree with renaming the method; it shouldn't be confused with the typical forEach() on a collection which visits *all* elements; here it only visits the remaining elements. >> >> Please review >> Thanks >> > > cheers, > Rémi >
