Skip to content

Commit f4e6e90

Browse files
kulvaitnormanrz
andcommitted
Apply suggestions from code review of Norman Rzepka
Thank you for the review, these suggestions integrate cleanly. I will review remaining suggestions and comment or integrate them. Co-authored-by: Norman Rzepka <code@normanrz.com>
1 parent b4cc32f commit f4e6e90

1 file changed

Lines changed: 4 additions & 15 deletions

File tree

src/main/java/dev/zarr/zarrjava/store/ReadOnlyZipStore.java

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
*/
3333
public class ReadOnlyZipStore extends ZipStore {
3434
private static final Logger logger = Logger.getLogger(ReadOnlyZipStore.class.getName());
35-
final Path zipStorePath; // Store the resolved zip path for logging and potential future use
35+
private final Path zipStorePath; // Store the resolved zip path for logging and potential future use
3636
// Cache keys are without trailing or leading slashes, leaf nodes are stored as their names
3737
private Map<String, Set<String>> directoryToChildrenDirectoriesIndex;
3838
private Map<String, Set<String>> directoryToChildrenFilesIndex;
@@ -60,17 +60,6 @@ public ReadOnlyZipStore(@Nonnull String zipPath) {
6060
this(Paths.get(zipPath));
6161
}
6262

63-
// Helper for buildZipIndex to add all parent directories of a given entry to the directory index, ensuring they are present for lookups
64-
private void addParentDirs(String entryName, Set<String> dirIndex) {
65-
int lastSlash = entryName.lastIndexOf('/'); // Find the last '/' in the file name
66-
while (lastSlash > 0) { // Keep going until no more slashes are found
67-
String parentDir = entryName.substring(0, lastSlash + 1); // Extract the parent directory path
68-
if (!dirIndex.add(parentDir)) { // Add the parent to the directory index if it’s not already added
69-
break; // Exit if this parent directory has already been added
70-
}
71-
lastSlash = entryName.lastIndexOf('/', lastSlash - 1); // Move the search for slashes up
72-
}
73-
}
7463

7564
// Helper for buildZipIndex to add a file entry to the file index and ensure all its parent directories are indexed as well
7665
private void insertLeafEntry(String entryStrippedPath, long size) {
@@ -282,7 +271,7 @@ private byte[] readEntryBytes(String[] keys, long start, long end) {
282271
public ByteBuffer get(String[] keys, long start, long end) {
283272
byte[] bytes = readEntryBytes(keys, start, end);
284273
if (bytes == null) {
285-
return ByteBuffer.allocate(0);
274+
return null;
286275
}
287276
return ByteBuffer.wrap(bytes);
288277
}
@@ -308,15 +297,15 @@ public String toString() {
308297
return "ReadOnlyZipStore(" + underlyingStore.toString() + ")";
309298
}
310299

311-
public static String[] concatPaths(String[] prefix, String[] child) {
300+
private static String[] concatPaths(String[] prefix, String[] child) {
312301
String[] result = new String[prefix.length + child.length];
313302
System.arraycopy(prefix, 0, result, 0, prefix.length);
314303
System.arraycopy(child, 0, result, prefix.length, child.length);
315304
return result;
316305
}
317306

318307

319-
public void addChildrenRecursively(String[] prefixZarrPath, String[] childrenZarrPath, Stream.Builder<String[]> builder) {
308+
private void addChildrenRecursively(String[] prefixZarrPath, String[] childrenZarrPath, Stream.Builder<String[]> builder) {
320309
String[] fullZarrPath = concatPaths(prefixZarrPath, childrenZarrPath);
321310

322311
String prefix = resolveKeys(fullZarrPath);

0 commit comments

Comments
 (0)