kvr000 commented on code in PR #378:
URL: https://github.com/apache/commons-compress/pull/378#discussion_r1438818531
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java:
##########
@@ -368,9 +369,14 @@ public static void closeQuietly(final ZipFile zipFile) {
private final List<ZipArchiveEntry> entries = new LinkedList<>();
/**
- * Maps String to list of ZipArchiveEntrys, name -> actual entries.
+ * Maps a string to the first entry by that name.
*/
- private final Map<String, LinkedList<ZipArchiveEntry>> nameMap = new
HashMap<>(HASH_SIZE);
+ private final Map<String, ZipArchiveEntry> nameMap = new
HashMap<>(HASH_SIZE);
+
+ /**
+ * If multiple entries have the same name, maps the name to entries by
that name.
+ */
+ private Map<String, List<ZipArchiveEntry>> duplicateNameMap;
Review Comment:
Given that this is extremely unlikely, would it make sense to keep the
original Map and change the value to Object and let it hold either
ZipArchiveEntry or the List? It would eventually complicate the lookup but
that should be a single place only.
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java:
##########
@@ -820,7 +839,13 @@ public Enumeration<ZipArchiveEntry> getEntries() {
* @since 1.6
*/
public Iterable<ZipArchiveEntry> getEntries(final String name) {
- return nameMap.getOrDefault(name, ZipArchiveEntry.EMPTY_LINKED_LIST);
Review Comment:
I think the slowdown is only in case there are duplicit entries which is
extremely unlikely. I'd still simplify the code, as according to earlier
comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
