If your objection is that I claimed that these qualify as "most of the cases" - I really don't know what else to say then. The original commit which did this change is this[1]. I haven't reviewed it fully, but the very first few changes that are done are these [2] [3] [4] [5][6].

Of course, there's a subsequent commit which then uses a different new util, instead of just using the existing iterator/enumeration. Speaking of the subsequent commit, it still doesn't undo the (IMO unnecessary) change that was done to some of the code (take a look at ArgumentProcessorRegistry.java for example).


Even if these don't fall under "most of the cases", why even change these places? I'm sure you would know this - the Enumeration or APIs that you refactored aren't even deprecated in Java version 10.

Anyway, I'm really getting tired of these back and forth arguments on refactoring. The reason I get involved in certain open source projects is to get a chance to work with like minded developers and learn something out of it and not to go wage a war on which coding style is better or try and be critical of other committers' commits. Unfortunately, in the recent past, this has reached a state where I have ended up spending more time being critical of changes that have gone in, than actually adding much code of value. As much as I try to stay away from reviews or checking the commit logs, I just keep going back to them. I don't want to end up being a grumpy guy criticizing the commits. I'm just going to take a break from this for some days and be a regular user and come back and see if I still enjoy contributing.

[1] https://github.com/apache/ant/commit/070c3bc86f85e8f01cb624fe50ae82f0d11171b2

[2] https://github.com/apache/ant/commit/070c3bc86f85e8f01cb624fe50ae82f0d11171b2#diff-21eb59eaf9f2b5d0b487aeb5e5022ccdL746 [3] https://github.com/apache/ant/commit/070c3bc86f85e8f01cb624fe50ae82f0d11171b2#diff-21eb59eaf9f2b5d0b487aeb5e5022ccdL834 [4] https://github.com/apache/ant/commit/070c3bc86f85e8f01cb624fe50ae82f0d11171b2#diff-21eb59eaf9f2b5d0b487aeb5e5022ccdL888 [5] https://github.com/apache/ant/commit/070c3bc86f85e8f01cb624fe50ae82f0d11171b2#diff-21eb59eaf9f2b5d0b487aeb5e5022ccdL1359

[6] https://github.com/apache/ant/commit/070c3bc86f85e8f01cb624fe50ae82f0d11171b2#diff-b98a3d2097d6a9b5d7e0fc2eac033f24L348


-Jaikiran


On 18/05/18 11:15 AM, Gintautas Grigelionis wrote:
I'm not quite sure that what you say was true "in most of the cases".

Gintas

2018-05-18 6:52 GMT+02:00 Jaikiran Pai <jai.forums2...@gmail.com>:

To be honest, I don't think this deprecation/conversion change is
good(including this recent commit whichintroduces a
StreamUtils.enumerationAsStream(...))

To give you an example, the code that was previously present would (in
most of the cases) get hold of an enumeration and would iterate over it and
during that iteration would runcertain logic. With the changes that went in
(which again is a bulk change!) the code now gets hold of an enumeration
and instead of just getting on the business of iterating and running
certain logic, instead now passes this enumeration aroundto convert it into
some other form (thus additional code plus additional objects allocated in
the process), all this to iterate over it and run some logic on it - all of
which was already possible with the enumeration that was already available.


-Jaikiran



On 18/05/18 12:22 AM, Gintautas Grigelionis wrote:

Thanks for reviewing, I hope Spliterators will do a better job.

Gintas

2018-05-17 8:37 GMT+02:00 Jaikiran Pai <jai.forums2...@gmail.com>:

I agree. Especially when it's being done on something like for archive
entries which can betoo many depending on the archive that is being dealt
with.

-Jaikiran



On 17/05/18 12:04 PM, Maarten Coene wrote:

Converting an Enumeration to a List just for iterating it doesn't seem
performance and memory wise a good idea to me.
Maarten

         Van: "gin...@apache.org" <gin...@apache.org>
    Aan: notificati...@ant.apache.org
    Verzonden: woensdag 16 mei 19:13 2018
    Onderwerp: [1/2] ant git commit: Deprecate CollectionUtils and
Enumerations; reduce explicit use of Enumeration
      Repository: ant
Updated Branches:
     refs/heads/master ac35c0014 -> 070c3bc86


http://git-wip-us.apache.org/repos/asf/ant/blob/070c3bc8/src
/main/org/apache/tools/ant/types/ZipScanner.java
----------------------------------------------------------------------
diff --git a/src/main/org/apache/tools/ant/types/ZipScanner.java
b/src/main/org/apache/tools/ant/types/ZipScanner.java
index a3df040..5667159 100644
--- a/src/main/org/apache/tools/ant/types/ZipScanner.java
+++ b/src/main/org/apache/tools/ant/types/ZipScanner.java
@@ -20,7 +20,7 @@ package org.apache.tools.ant.types;
      import java.io.File;
    import java.io.IOException;
-import java.util.Enumeration;
+import java.util.Collections;
    import java.util.Map;
    import java.util.zip.ZipException;
    @@ -62,10 +62,7 @@ public class ZipScanner extends ArchiveScanner {
                   "Only file provider resources are supported"));
             try (ZipFile zf = new ZipFile(srcFile, encoding)) {
-
-            Enumeration<ZipEntry> e = zf.getEntries();
-            while (e.hasMoreElements()) {
-                ZipEntry entry = e.nextElement();
+            for (ZipEntry entry : Collections.list(zf.getEntries())) {
                   Resource r = new ZipResource(srcFile, encoding,
entry);
                   String name = entry.getName();
                   if (entry.isDirectory()) {





---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

Reply via email to