Re: (commons-compress) branch master updated: Replace org.apache.commons.io.Charsets with org.apache.commons.compress.utils.Charsets

[Gary Gregory]

Emanuel,

-1 you are duplicating code (again) as a one-off while adding the clutter
of a ternary expression (and unnecessary parentheses). It is much cleaner
to use canonical-like code from our low level Commons IO library we already
use elsewhere. There is no benefit to expanding this API call. This change
forces the reader to parse out the ternary expreasion, the how, instead of
focusing on the what of the method.

TY,
Gary



On Fri, Nov 1, 2024, 1:55 PM  wrote:

> This is an automated email from the ASF dual-hosted git repository.
>
> ebourg pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/commons-compress.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
>  new 5d2456748 Replace org.apache.commons.io.Charsets with
> org.apache.commons.compress.utils.Charsets
> 5d2456748 is described below
>
> commit 5d24567489996a910af842d5db6ed52ded54e1c1
> Author: Emmanuel Bourg 
> AuthorDate: Fri Nov 1 18:22:35 2024 +0100
>
> Replace org.apache.commons.io.Charsets with
> org.apache.commons.compress.utils.Charsets
> ---
>  .../org/apache/commons/compress/archivers/ArchiveInputStream.java | 2
> +-
>  .../apache/commons/compress/archivers/tar/TarArchiveOutputStream.java | 2
> +-
>  .../apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java | 2
> +-
>  .../org/apache/commons/compress/archivers/zip/ZipEncodingHelper.java  | 2
> +-
>  src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java  | 4
> ++--
>  5 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git
> a/src/main/java/org/apache/commons/compress/archivers/ArchiveInputStream.java
> b/src/main/java/org/apache/commons/compress/archivers/ArchiveInputStream.java
> index d8f312faf..2a007d050 100644
> ---
> a/src/main/java/org/apache/commons/compress/archivers/ArchiveInputStream.java
> +++
> b/src/main/java/org/apache/commons/compress/archivers/ArchiveInputStream.java
> @@ -25,7 +25,7 @@ import java.nio.charset.Charset;
>  import java.util.Iterator;
>  import java.util.Objects;
>
> -import org.apache.commons.io.Charsets;
> +import org.apache.commons.compress.utils.Charsets;
>  import org.apache.commons.io.function.IOConsumer;
>  import org.apache.commons.io.function.IOIterator;
>  import org.apache.commons.io.input.NullInputStream;
> diff --git
> a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
> b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
> index 4f3588d9b..2c5878e83 100644
> ---
> a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
> +++
> b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
> @@ -38,9 +38,9 @@ import java.util.Map;
>  import org.apache.commons.compress.archivers.ArchiveOutputStream;
>  import org.apache.commons.compress.archivers.zip.ZipEncoding;
>  import org.apache.commons.compress.archivers.zip.ZipEncodingHelper;
> +import org.apache.commons.compress.utils.Charsets;
>  import org.apache.commons.compress.utils.FixedLengthBlockOutputStream;
>  import org.apache.commons.compress.utils.TimeUtils;
> -import org.apache.commons.io.Charsets;
>  import org.apache.commons.io.file.attribute.FileTimes;
>  import org.apache.commons.io.output.CountingOutputStream;
>  import org.apache.commons.lang3.ArrayFill;
> diff --git
> a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java
> b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java
> index 0849f2848..f617c2b7e 100644
> ---
> a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java
> +++
> b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java
> @@ -38,7 +38,7 @@ import java.util.zip.ZipException;
>  import org.apache.commons.compress.archivers.ArchiveEntry;
>  import org.apache.commons.compress.archivers.ArchiveOutputStream;
>  import org.apache.commons.compress.utils.ByteUtils;
> -import org.apache.commons.io.Charsets;
> +import org.apache.commons.compress.utils.Charsets;
>
>  /**
>   * Reimplementation of {@link java.util.zip.ZipOutputStream
> java.util.zip.ZipOutputStream} to handle the extended functionality of this
> package, especially
> diff --git
> a/src/main/java/org/apache/commons/compress/archivers/zip/ZipEncodingHelper.java
> b/src/main/java/org/apache/commons/compress/archivers/zip/ZipEncodingHelper.java
> index 3f4dede20..8ba45f801 100644
> ---
> a/src/main/java/org/apache/commons/compress/archivers/zip/ZipEncodingHelper.java
> +++
> b/src/main/java/org/apache/commons/compress/archivers/zip/ZipEncodingHelper.java
> @@ -23,7 +23,7 @@ import java.nio.ByteBuffer;
>  import java.nio.charset.Charset;
>  import java.nio.charset.UnsupportedCharsetException;
>
> -import org.apache.commons.io.Charsets;
> +import org.apache.commons.compress.utils.Charsets;
>
>  /**
>   * Static helper functions for robustly encoding file names in ZIP

Re: Request for Tracking Byte Information in CSV Records

[Gary Gregory]

Hello Yuzhan,

I replied in one of the tickets where has been a discussion going on for a
while.

The use of an encoder or decoder should be optional as this would have a
possible negative effect on performance.

I am not sure we should consider flipping the processing and base it on an
input stream instead of a reader to get access to bytes before we would
have to convert them to characters, that seems a radical change.

Gary

On Tue, Oct 29, 2024, 10:13 AM Yuzhan Jiang  wrote:

> Dear Apache Commons Development Team,
>
> I hope this message finds you well. I am reaching out to discuss an
> enhancement request for tracking byte information in CSV records parsed
> from CSV files.
>
> Currently, many users face challenges due to misunderstandings between
> bytes and characters. This is especially problematic in situations where
> multi-byte characters are present, and the appropriate character encoding
> has not been applied. These issues often arise when bytes are mistakenly
> treated as characters, assuming a one-to-one correspondence.
>
> If there is agreement that this enhancement would benefit the community, I
> suggest we revisit the discussions on this topic, specifically around Matt
> Sun’s work on
> [CSV-196] Store the information of raw data read by lexer - ASF JIRA <
> https://issues.apache.org/jira/browse/CSV-196>  and the PR that he had
> submitted[CSV-196] Track byte information of the source by mattsunsjf ·
> Pull Request #22 · apache/commons-cs <
> https://github.com/apache/commons-csv/pull/22>any other similar issues
> that have been raised
>  [CSV-229] Allow byte
> position tracking in CSVParser - ASF JIRA <
> https://issues.apache.org/jira/browse/CSV-229>, so that we can just use
> the standard commons-csv libraries.
>
> Thank you for considering this request, and I look forward to any thoughts
> or feedback from the community.
>
> Best regards,
> Yuzhan

Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3

[Gary D. Gregory]

There has been no revert from the committer since the -1 on October 24, so I 
will revert. This affects 11 Java files. This will revert the copy and pasting 
of code patterns over and over.

Gary

On 2024/10/24 21:17:41 Emmanuel Bourg wrote:
> Le 24/10/2024 à 21:05, Gary Gregory a écrit :
> 
> > Uh, no, don't be disingenuous by showing 3 lines, the commit is many pages
> > long of diff. All of it is -1 as it increases the code complexity, increase
> > code duplication, and adds the anti-pattern of throwing RuntimeException
> > more than once.
> 
> My commit message could have been clearer, it actually reverted the 
> commit 87e898fa [1] and replaced the reflection code with the snippet I 
> mentioned. So the code doesn't come out of my hat, that's what 
> commons-compress already had for years before.
> 
> Adding a dependency such as commons-lang3 has a significant impact for 
> the users. In my case it broke an application that was using 
> commons-compress but didn't have commons-lang3 on the classpath.
> 
> I understand your desire to reuse as much code as possible to ease the 
> maintenance, but in this case the user's convenience outweighs the 
> maintainer's convenience. commons-lang3 doesn't bring enough value in 
> this case to justify its addition to the runtime dependencies.
> 
> Emmanuel Bourg
> 
> [1] https://github.com/apache/commons-compress/commit/87e898fa
> 
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
> 
> 

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

Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3

[sebb]

On Fri, 1 Nov 2024 at 14:22, Emmanuel Bourg  wrote:
>
> Hi Gary,
>
> Le 01/11/2024 à 14:22, Gary D. Gregory a écrit :
> > There has been no revert from the committer since the -1 on October 24, so 
> > I will revert. This affects 11 Java files. This will revert the copy and 
> > pasting of code patterns over and over.
>
> I don't understand your "copy pasting" argument, the commit replaced
> exactly 8 occurrences of:
>
>int[] array = ArrayFill.fill(new int[length], value);
>
> with:
>
>int[] array = new int[length];
>Arrays.fill(array, value);
>
>
> It also replaced:
>
>SystemProperties.getOsName();
>
> with:
>
>System.getProperty("os.name");
>
>
> And two reflexive calls such as:
>
>FieldUtils.readField(object, fieldName, true);
>
> with:
>
>Field field = InputStream.class.getDeclaredField(fieldName);
>field.setAccessible(true);
>return field.get(object);
>
>
> These are not unreasonable substitutions to avoid a 660KB dependency.
> You make it sound like I copy-pasted tons of lines, but that's only 6
> more lines of code overall.
>
>
> Let me put this differently, I'm -1 to your commits bf50b7d9 [1],
> 87e898fa [2] and 09031c91 [3] from earlier this year that introduced the
> dependency on Commons Lang in the first place. I'm especially unhappy
> with the first one, a mix of unrelated changes confusingly labeled as
> "Internal refactoring",  where the dependency sneaked into without
> notice, neither mentioned in the commit message nor in the changelog.
> This dependency is an unnecessary bloat.

+1

> I'm also concerned about the extra dependency on Commons Codec and
> Commons IO, also added this year. I'd like to return to the state of
> Commons Compress 1.25 with *zero* runtime dependency (besides the
> optional ones required for the various compression algorithms
> supported). For Codec and IO I suggest shading the classes as a compromise.
>
> Emmanuel Bourg
>
> [1] https://github.com/apache/commons-compress/commit/bf50b7d9
> [2] https://github.com/apache/commons-compress/commit/87e898fa
> [3] https://github.com/apache/commons-compress/commit/09031c91
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>

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

Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3

[Richard Zowalla]

From the sideline: I am also concerned about adding dependendcies in low level 
libraries for replacing only a few lines of code.

 From ASF policy perspective, the veto (-1) requires a technical explanation, 
cf. https://www.apache.org/foundation/voting.html#Veto

"To prevent vetoes from being used capriciously, the voter must provide with 
the veto a technical justification showing why the change is bad (opens a 
security exposure, negatively affects performance, etc. ). A veto without a 
justification is invalid and has no weight."

So IMHO asking for an explanation about the actual CnP is totally fine.

Gruß 
Richard 



Am 1. November 2024 15:22:07 MEZ schrieb Emmanuel Bourg :
>Hi Gary,
>
>Le 01/11/2024 à 14:22, Gary D. Gregory a écrit :
>> There has been no revert from the committer since the -1 on October 24, so I 
>> will revert. This affects 11 Java files. This will revert the copy and 
>> pasting of code patterns over and over.
>
>I don't understand your "copy pasting" argument, the commit replaced exactly 8 
>occurrences of:
>
>  int[] array = ArrayFill.fill(new int[length], value);
>
>with:
>
>  int[] array = new int[length];
>  Arrays.fill(array, value);
>
>
>It also replaced:
>
>  SystemProperties.getOsName();
>
>with:
>
>  System.getProperty("os.name");
>
>
>And two reflexive calls such as:
>
>  FieldUtils.readField(object, fieldName, true);
>
>with:
>
>  Field field = InputStream.class.getDeclaredField(fieldName);
>  field.setAccessible(true);
>  return field.get(object);
>
>
>These are not unreasonable substitutions to avoid a 660KB dependency. You make 
>it sound like I copy-pasted tons of lines, but that's only 6 more lines of 
>code overall.
>
>
>Let me put this differently, I'm -1 to your commits bf50b7d9 [1], 87e898fa [2] 
>and 09031c91 [3] from earlier this year that introduced the dependency on 
>Commons Lang in the first place. I'm especially unhappy with the first one, a 
>mix of unrelated changes confusingly labeled as "Internal refactoring",  where 
>the dependency sneaked into without notice, neither mentioned in the commit 
>message nor in the changelog. This dependency is an unnecessary bloat.
>
>I'm also concerned about the extra dependency on Commons Codec and Commons IO, 
>also added this year. I'd like to return to the state of Commons Compress 1.25 
>with *zero* runtime dependency (besides the optional ones required for the 
>various compression algorithms supported). For Codec and IO I suggest shading 
>the classes as a compromise.
>
>Emmanuel Bourg
>
>[1] https://github.com/apache/commons-compress/commit/bf50b7d9
>[2] https://github.com/apache/commons-compress/commit/87e898fa
>[3] https://github.com/apache/commons-compress/commit/09031c91
>
>
>-
>To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>For additional commands, e-mail: dev-h...@commons.apache.org
>

Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3

[Emmanuel Bourg]

Hi Gary,

Le 01/11/2024 à 14:22, Gary D. Gregory a écrit :

There has been no revert from the committer since the -1 on October 24, so I 
will revert. This affects 11 Java files. This will revert the copy and pasting 
of code patterns over and over.


I don't understand your "copy pasting" argument, the commit replaced 
exactly 8 occurrences of:


  int[] array = ArrayFill.fill(new int[length], value);

with:

  int[] array = new int[length];
  Arrays.fill(array, value);


It also replaced:

  SystemProperties.getOsName();

with:

  System.getProperty("os.name");


And two reflexive calls such as:

  FieldUtils.readField(object, fieldName, true);

with:

  Field field = InputStream.class.getDeclaredField(fieldName);
  field.setAccessible(true);
  return field.get(object);


These are not unreasonable substitutions to avoid a 660KB dependency. 
You make it sound like I copy-pasted tons of lines, but that's only 6 
more lines of code overall.



Let me put this differently, I'm -1 to your commits bf50b7d9 [1], 
87e898fa [2] and 09031c91 [3] from earlier this year that introduced the 
dependency on Commons Lang in the first place. I'm especially unhappy 
with the first one, a mix of unrelated changes confusingly labeled as 
"Internal refactoring",  where the dependency sneaked into without 
notice, neither mentioned in the commit message nor in the changelog. 
This dependency is an unnecessary bloat.


I'm also concerned about the extra dependency on Commons Codec and 
Commons IO, also added this year. I'd like to return to the state of 
Commons Compress 1.25 with *zero* runtime dependency (besides the 
optional ones required for the various compression algorithms 
supported). For Codec and IO I suggest shading the classes as a compromise.


Emmanuel Bourg

[1] https://github.com/apache/commons-compress/commit/bf50b7d9
[2] https://github.com/apache/commons-compress/commit/87e898fa
[3] https://github.com/apache/commons-compress/commit/09031c91


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

Re: (commons-compress) branch master updated: Replace org.apache.commons.io.Charsets with org.apache.commons.compress.utils.Charsets

[Richard Zowalla]

Maybe it's time to start a separate discussion thread about the addition of the 
three libraries to compress after 1.25?

Otherwise, I fear, that we will continue to see -1 and code reverts over and 
over again because there seems to be no consensus here.

So maybe let's frame it as a discussiom thread on the list and find a 
consensus-based solution? 

IMHO (from a consumer pov) the addition of >1mb of dependencies is unfortunate.


Gruß 
Richard 

Am 1. November 2024 20:04:33 MEZ schrieb Gary Gregory :
>Emanuel,
>
>-1 you are duplicating code (again) as a one-off while adding the clutter
>of a ternary expression (and unnecessary parentheses). It is much cleaner
>to use canonical-like code from our low level Commons IO library we already
>use elsewhere. There is no benefit to expanding this API call. This change
>forces the reader to parse out the ternary expreasion, the how, instead of
>focusing on the what of the method.
>
>TY,
>Gary
>
>
>
>On Fri, Nov 1, 2024, 1:55 PM  wrote:
>
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> ebourg pushed a commit to branch master
>> in repository https://gitbox.apache.org/repos/asf/commons-compress.git
>>
>>
>> The following commit(s) were added to refs/heads/master by this push:
>>  new 5d2456748 Replace org.apache.commons.io.Charsets with
>> org.apache.commons.compress.utils.Charsets
>> 5d2456748 is described below
>>
>> commit 5d24567489996a910af842d5db6ed52ded54e1c1
>> Author: Emmanuel Bourg 
>> AuthorDate: Fri Nov 1 18:22:35 2024 +0100
>>
>> Replace org.apache.commons.io.Charsets with
>> org.apache.commons.compress.utils.Charsets
>> ---
>>  .../org/apache/commons/compress/archivers/ArchiveInputStream.java | 2
>> +-
>>  .../apache/commons/compress/archivers/tar/TarArchiveOutputStream.java | 2
>> +-
>>  .../apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java | 2
>> +-
>>  .../org/apache/commons/compress/archivers/zip/ZipEncodingHelper.java  | 2
>> +-
>>  src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java  | 4
>> ++--
>>  5 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git
>> a/src/main/java/org/apache/commons/compress/archivers/ArchiveInputStream.java
>> b/src/main/java/org/apache/commons/compress/archivers/ArchiveInputStream.java
>> index d8f312faf..2a007d050 100644
>> ---
>> a/src/main/java/org/apache/commons/compress/archivers/ArchiveInputStream.java
>> +++
>> b/src/main/java/org/apache/commons/compress/archivers/ArchiveInputStream.java
>> @@ -25,7 +25,7 @@ import java.nio.charset.Charset;
>>  import java.util.Iterator;
>>  import java.util.Objects;
>>
>> -import org.apache.commons.io.Charsets;
>> +import org.apache.commons.compress.utils.Charsets;
>>  import org.apache.commons.io.function.IOConsumer;
>>  import org.apache.commons.io.function.IOIterator;
>>  import org.apache.commons.io.input.NullInputStream;
>> diff --git
>> a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
>> b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
>> index 4f3588d9b..2c5878e83 100644
>> ---
>> a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
>> +++
>> b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
>> @@ -38,9 +38,9 @@ import java.util.Map;
>>  import org.apache.commons.compress.archivers.ArchiveOutputStream;
>>  import org.apache.commons.compress.archivers.zip.ZipEncoding;
>>  import org.apache.commons.compress.archivers.zip.ZipEncodingHelper;
>> +import org.apache.commons.compress.utils.Charsets;
>>  import org.apache.commons.compress.utils.FixedLengthBlockOutputStream;
>>  import org.apache.commons.compress.utils.TimeUtils;
>> -import org.apache.commons.io.Charsets;
>>  import org.apache.commons.io.file.attribute.FileTimes;
>>  import org.apache.commons.io.output.CountingOutputStream;
>>  import org.apache.commons.lang3.ArrayFill;
>> diff --git
>> a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java
>> b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java
>> index 0849f2848..f617c2b7e 100644
>> ---
>> a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java
>> +++
>> b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java
>> @@ -38,7 +38,7 @@ import java.util.zip.ZipException;
>>  import org.apache.commons.compress.archivers.ArchiveEntry;
>>  import org.apache.commons.compress.archivers.ArchiveOutputStream;
>>  import org.apache.commons.compress.utils.ByteUtils;
>> -import org.apache.commons.io.Charsets;
>> +import org.apache.commons.compress.utils.Charsets;
>>
>>  /**
>>   * Reimplementation of {@link java.util.zip.ZipOutputStream
>> java.util.zip.ZipOutputStream} to handle the extended functionality of this
>> package, especially
>> diff --git
>> a/src/main/java/org/apache/commons/compress/archivers/zip/ZipEncodingHelper.java

Re: (commons-compress) branch master updated: Replace org.apache.commons.io.Charsets with org.apache.commons.compress.utils.Charsets

[Emmanuel Bourg]

Le 01/11/2024 à 20:04, Gary Gregory a écrit :

Emanuel,

-1 you are duplicating code (again) as a one-off while adding the clutter
of a ternary expression (and unnecessary parentheses). It is much cleaner
to use canonical-like code from our low level Commons IO library we already
use elsewhere. There is no benefit to expanding this API call. This change
forces the reader to parse out the ternary expreasion, the how, instead of
focusing on the what of the method.


Gary,

There are 4 other ternary expressions in the ZipFile class, 90+ in the 
zip package and over 270 in the whole code, not counting the unit tests. 
Some of these expressions were added by you this year, in commits 
labeled "Use ternary expression" [1][2][3]. So I conclude that it's 
acceptable to use this syntax.


The extra parentheses improve the readability in my opinion, but I don't 
mind removing them.


Expanding the API call is necessary to remove the undesired dependency 
on Commons IO.


Emmanuel Bourg

[1] https://github.com/apache/commons-compress/commit/bbe6fd9a
[2] https://github.com/apache/commons-compress/commit/aa71cb93
[3] https://github.com/apache/commons-compress/commit/b3e34e89


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

Re: (commons-compress) branch master updated: Replace org.apache.commons.io.Charsets with org.apache.commons.compress.utils.Charsets

[Gary Gregory]

Emanuel and all,

I see dependencies here and in other libraries a natural aspect of Java
development in that it helps reuse proven solutions, where bug fixes and
implementations are improved, while externalizing that cost to the library
which is otherwise born by the host code base.

Reusing a library allows the host component to focus more on its business
than having to draw attention from it. The benefits far outweigh any
drawbacks in my view.

In addition, reuse makes the code easier to read, easier to maintain, and
easier to contribute to without getting lost in the weeds of code that's
been copy pasted all over the place.

A mention was made of the size on disk of jar files, this feels immaterial
when a JVM is now very good at only loading the classes it needs out of a
jar file.

HTH,
Gary

On Fri, Nov 1, 2024, 5:53 PM Emmanuel Bourg  wrote:

> Le 01/11/2024 à 20:04, Gary Gregory a écrit :
> > Emanuel,
> >
> > -1 you are duplicating code (again) as a one-off while adding the clutter
> > of a ternary expression (and unnecessary parentheses). It is much cleaner
> > to use canonical-like code from our low level Commons IO library we
> already
> > use elsewhere. There is no benefit to expanding this API call. This
> change
> > forces the reader to parse out the ternary expreasion, the how, instead
> of
> > focusing on the what of the method.
>
> Gary,
>
> There are 4 other ternary expressions in the ZipFile class, 90+ in the
> zip package and over 270 in the whole code, not counting the unit tests.
> Some of these expressions were added by you this year, in commits
> labeled "Use ternary expression" [1][2][3]. So I conclude that it's
> acceptable to use this syntax.
>
> The extra parentheses improve the readability in my opinion, but I don't
> mind removing them.
>
> Expanding the API call is necessary to remove the undesired dependency
> on Commons IO.
>
> Emmanuel Bourg
>
> [1] https://github.com/apache/commons-compress/commit/bbe6fd9a
> [2] https://github.com/apache/commons-compress/commit/aa71cb93
> [3] https://github.com/apache/commons-compress/commit/b3e34e89
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>

Re: (commons-compress) branch master updated: Replace org.apache.commons.io.Charsets with org.apache.commons.compress.utils.Charsets

[sebb]

On Fri, 1 Nov 2024 at 22:27, Gary Gregory  wrote:
>
> Emanuel and all,
>
> I see dependencies here and in other libraries a natural aspect of Java
> development in that it helps reuse proven solutions, where bug fixes and
> implementations are improved, while externalizing that cost to the library
> which is otherwise born by the host code base.
>
> Reusing a library allows the host component to focus more on its business
> than having to draw attention from it. The benefits far outweigh any
> drawbacks in my view.

That would be a reasonable argument if a substantial amount of code
were involved.
That is clearly not the case here.

> In addition, reuse makes the code easier to read, easier to maintain, and
> easier to contribute to without getting lost in the weeds of code that's
> been copy pasted all over the place.

That is not the case here.

> A mention was made of the size on disk of jar files, this feels immaterial
> when a JVM is now very good at only loading the classes it needs out of a
> jar file.
>
> HTH,
> Gary
>
> On Fri, Nov 1, 2024, 5:53 PM Emmanuel Bourg  wrote:
>
> > Le 01/11/2024 à 20:04, Gary Gregory a écrit :
> > > Emanuel,
> > >
> > > -1 you are duplicating code (again) as a one-off while adding the clutter
> > > of a ternary expression (and unnecessary parentheses). It is much cleaner
> > > to use canonical-like code from our low level Commons IO library we
> > already
> > > use elsewhere. There is no benefit to expanding this API call. This
> > change
> > > forces the reader to parse out the ternary expreasion, the how, instead
> > of
> > > focusing on the what of the method.
> >
> > Gary,
> >
> > There are 4 other ternary expressions in the ZipFile class, 90+ in the
> > zip package and over 270 in the whole code, not counting the unit tests.
> > Some of these expressions were added by you this year, in commits
> > labeled "Use ternary expression" [1][2][3]. So I conclude that it's
> > acceptable to use this syntax.
> >
> > The extra parentheses improve the readability in my opinion, but I don't
> > mind removing them.
> >
> > Expanding the API call is necessary to remove the undesired dependency
> > on Commons IO.
> >
> > Emmanuel Bourg
> >
> > [1] https://github.com/apache/commons-compress/commit/bbe6fd9a
> > [2] https://github.com/apache/commons-compress/commit/aa71cb93
> > [3] https://github.com/apache/commons-compress/commit/b3e34e89
> >
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> > For additional commands, e-mail: dev-h...@commons.apache.org
> >
> >

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

Re: (commons-compress) branch master updated: Replace org.apache.commons.io.Charsets with org.apache.commons.compress.utils.Charsets

[Emmanuel Bourg]

Le 01/11/2024 à 21:12, Richard Zowalla a écrit :


IMHO (from a consumer pov) the addition of >1mb of dependencies is unfortunate.


commons-codec 1.17.1354KB
commons-lang  3.17.0658KB
commons-io2.17.0504KB

Total: 1.5MB :(

Emmanuel Bourg


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