PackIndex: Simplify Iterator/MutableEntry interaction
The iterator keeps the current position in the index and the MutableEntry reads data from there on-demand, but the iterator needs to know about the entry and this creates a complicated interaction.
Make MutableEntry a simple data object and let the iterator iterate and populate it before returning it. Code is clearer and implementors only needs to worry about the iterator.
This fixes also MutableEntry visibility, that was preventing subclassing from out of the package.
Change-Id: I35010d1f80237e421dd51b8d3d61a8ecb03e0d01
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndex.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndex.java
index c0540d5..95050d4 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndex.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndex.java
@@ -302,8 +302,9 @@
*
*/
class MutableEntry {
+ /** Buffer of the ObjectId visited by the EntriesIterator. */
final MutableObjectId idBuffer = new MutableObjectId();
-
+ /** Offset into the packfile of the current object. */
long offset;
/**
@@ -321,7 +322,6 @@
* @return hex string describing the object id of this entry.
*/
public String name() {
- ensureId();
return idBuffer.name();
}
@@ -331,7 +331,6 @@
* @return a copy of the object id.
*/
public ObjectId toObjectId() {
- ensureId();
return idBuffer.toObjectId();
}
@@ -342,32 +341,32 @@
*/
public MutableEntry cloneEntry() {
final MutableEntry r = new MutableEntry();
- ensureId();
r.idBuffer.fromObjectId(idBuffer);
r.offset = offset;
return r;
}
-
- void ensureId() {
- // Override in implementations.
- }
}
/**
* Base implementation of the iterator over index entries.
*/
abstract class EntriesIterator implements Iterator<MutableEntry> {
- protected final MutableEntry entry = initEntry();
-
private final long objectCount;
+ private final MutableEntry entry = new MutableEntry();
+
+ /** Counts number of entries accessed so far. */
+ private long returnedNumber = 0;
+
+ /**
+ * Default constructor.
+ *
+ * @param objectCount the number of objects in the PackFile.
+ */
protected EntriesIterator(long objectCount) {
this.objectCount = objectCount;
}
- protected long returnedNumber = 0;
-
- protected abstract MutableEntry initEntry();
@Override
public boolean hasNext() {
@@ -379,7 +378,18 @@
* element.
*/
@Override
- public abstract MutableEntry next();
+ public MutableEntry next() {
+ readNext(entry);
+ returnedNumber++;
+ return entry;
+ }
+
+ /**
+ * Used by subclasses to load the next entry into the MutableEntry.
+ *
+ * @param entry the container of the next Iterator entry.
+ */
+ protected abstract void readNext(MutableEntry entry);
@Override
public void remove() {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndexV1.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndexV1.java
index d7c8378..99f3315 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndexV1.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndexV1.java
@@ -203,7 +203,7 @@
@Override
public Iterator<MutableEntry> iterator() {
- return new IndexV1Iterator(objectCnt);
+ return new EntriesIteratorV1(this);
}
@Override
@@ -246,36 +246,30 @@
return packChecksum;
}
- private class IndexV1Iterator extends EntriesIterator {
- int levelOne;
+ private static class EntriesIteratorV1 extends EntriesIterator {
+ private int levelOne;
- int levelTwo;
+ private int levelTwo;
- IndexV1Iterator(long objectCount) {
- super(objectCount);
+ private final PackIndexV1 packIndex;
+
+ private EntriesIteratorV1(PackIndexV1 packIndex) {
+ super(packIndex.objectCnt);
+ this.packIndex = packIndex;
}
@Override
- protected MutableEntry initEntry() {
- return new MutableEntry() {
- @Override
- protected void ensureId() {
- idBuffer.fromRaw(idxdata[levelOne], levelTwo
- - Constants.OBJECT_ID_LENGTH);
- }
- };
- }
-
- @Override
- public MutableEntry next() {
- for (; levelOne < idxdata.length; levelOne++) {
- if (idxdata[levelOne] == null)
+ protected void readNext(MutableEntry entry) {
+ for (; levelOne < packIndex.idxdata.length; levelOne++) {
+ if (packIndex.idxdata[levelOne] == null)
continue;
- if (levelTwo < idxdata[levelOne].length) {
- entry.offset = NB.decodeUInt32(idxdata[levelOne], levelTwo);
- levelTwo += Constants.OBJECT_ID_LENGTH + 4;
- returnedNumber++;
- return entry;
+ if (levelTwo < packIndex.idxdata[levelOne].length) {
+ entry.offset = NB.decodeUInt32(packIndex.idxdata[levelOne],
+ levelTwo);
+ this.levelTwo += Constants.OBJECT_ID_LENGTH + 4;
+ entry.idBuffer.fromRaw(packIndex.idxdata[levelOne],
+ levelTwo - Constants.OBJECT_ID_LENGTH);
+ return;
}
levelTwo = 0;
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndexV2.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndexV2.java
index caf8b71..f23380d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndexV2.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndexV2.java
@@ -224,7 +224,7 @@
@Override
public Iterator<MutableEntry> iterator() {
- return new EntriesIteratorV2(objectCnt);
+ return new EntriesIteratorV2(this);
}
@Override
@@ -289,41 +289,34 @@
return packChecksum;
}
- private class EntriesIteratorV2 extends EntriesIterator {
- int levelOne;
+ private static class EntriesIteratorV2 extends EntriesIterator {
+ private int levelOne = 0;
- int levelTwo;
+ private int levelTwo = 0;
- EntriesIteratorV2(long objectCount){
- super(objectCount);
+ private final PackIndexV2 packIndex;
+
+ private EntriesIteratorV2(PackIndexV2 packIndex) {
+ super(packIndex.objectCnt);
+ this.packIndex = packIndex;
}
@Override
- protected MutableEntry initEntry() {
- return new MutableEntry() {
- @Override
- protected void ensureId() {
- idBuffer.fromRaw(names[levelOne], levelTwo
- - Constants.OBJECT_ID_LENGTH / 4);
- }
- };
- }
-
- @Override
- public MutableEntry next() {
- for (; levelOne < names.length; levelOne++) {
- if (levelTwo < names[levelOne].length) {
+ protected void readNext(MutableEntry entry) {
+ for (; levelOne < packIndex.names.length; levelOne++) {
+ if (levelTwo < packIndex.names[levelOne].length) {
int idx = levelTwo / (Constants.OBJECT_ID_LENGTH / 4) * 4;
- long offset = NB.decodeUInt32(offset32[levelOne], idx);
+ long offset = NB.decodeUInt32(packIndex.offset32[levelOne],
+ idx);
if ((offset & IS_O64) != 0) {
idx = (8 * (int) (offset & ~IS_O64));
- offset = NB.decodeUInt64(offset64, idx);
+ offset = NB.decodeUInt64(packIndex.offset64, idx);
}
entry.offset = offset;
-
- levelTwo += Constants.OBJECT_ID_LENGTH / 4;
- returnedNumber++;
- return entry;
+ this.levelTwo += Constants.OBJECT_ID_LENGTH / 4;
+ entry.idBuffer.fromRaw(packIndex.names[levelOne],
+ levelTwo - Constants.OBJECT_ID_LENGTH / 4);
+ return;
}
levelTwo = 0;
}
