ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jaikiran Pai <jai.forums2...@gmail.com>
Subject Re: [1/2] ant git commit: Deprecate CollectionUtils and Enumerations; reduce explicit use of Enumeration
Date Fri, 18 May 2018 06:30:26 GMT
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.forums2013@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.forums2013@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: "gintas@apache.org" <gintas@apache.org>
>>>>>     Aan: notifications@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-unsubscribe@ant.apache.org
>>>> For additional commands, e-mail: dev-help@ant.apache.org
>>>>
>>>>
>>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
>> For additional commands, e-mail: dev-help@ant.apache.org
>>
>>


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


Mime
View raw message