PeterAlfredLee commented on a change in pull request #120:
URL: https://github.com/apache/commons-compress/pull/120#discussion_r468531249



##########
File path: 
src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java
##########
@@ -997,6 +1000,9 @@ private void readFilesInfo(final ByteBuffer header, final 
Archive archive) throw
                         throw new IOException("Unimplemented");
                     }
                     for (int i = 0; i < files.length; i++) {
+                        if (files[i] == null) {
+                            files[i] = new SevenZArchiveEntry();
+                        }

Review comment:
       Agree with @garydgregory about this duplicate code. We can have a simple 
refactor here.

##########
File path: 
src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java
##########
@@ -1090,7 +1108,13 @@ private void readFilesInfo(final ByteBuffer header, 
final Archive archive) throw
                 ++emptyFileCounter;
             }
         }
-        archive.files = files;
+        List<SevenZArchiveEntry> entries = new ArrayList<>();

Review comment:
       Maybe a final here?
   ```
   final List<SevenZArchiveEntry> entries = new ArrayList<>();
   ```

##########
File path: 
src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java
##########
@@ -981,8 +979,13 @@ private void readFilesInfo(final ByteBuffer header, final 
Archive archive) throw
                     int nextName = 0;
                     for (int i = 0; i < names.length; i += 2) {
                         if (names[i] == 0 && names[i+1] == 0) {
-                            files[nextFile++].setName(new String(names, 
nextName, i-nextName, StandardCharsets.UTF_16LE));
+                            String fName = new String(names, nextName, i - 
nextName, StandardCharsets.UTF_16LE);
+                            if (files[nextFile] == null) {
+                                files[nextFile] = new SevenZArchiveEntry();
+                            }
+                            files[nextFile].setName(fName);

Review comment:
       I think we do not need a variable `fName` here :
   
   ```
   if (files[nextFile] == null) {
       files[nextFile] = new SevenZArchiveEntry();
   }
   files[nextFile].setName(new String(names, nextName, i - nextName, 
StandardCharsets.UTF_16LE));
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to