Skip to content

Commit

Permalink
Fixes moves calculation to produce only minimal required moves
Browse files Browse the repository at this point in the history
  • Loading branch information
Nickolay Tarbayev committed Mar 27, 2018
1 parent df18195 commit 96f1a43
Show file tree
Hide file tree
Showing 2 changed files with 267 additions and 14 deletions.
87 changes: 85 additions & 2 deletions Source/Common/IGListDiff.mm
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,65 @@ static void addIndexToCollection(BOOL useIndexPaths, __unsafe_unretained id coll
return paths;
}

// Calculates longest increasing indexes set using O(n log n) complexity algorithm
static NSIndexSet *longestIncreasingIndexes(vector<IGListRecord> newResultsArray)
{
NSUInteger count = newResultsArray.size();
vector<NSUInteger> prevIndexes(count);
vector<NSUInteger> indexes(count + 1);

NSUInteger length = 0;
for (NSUInteger i = 0; i < count; i++) {
// Binary search for the largest positive j ≤ length
// such that X[M[j]] < X[i]
NSUInteger lo = 1;
NSUInteger hi = length;
while (lo <= hi) {
auto mid = ceil((lo + hi) / 2);
if (newResultsArray[indexes[mid]].index < newResultsArray[i].index) {
lo = mid + 1;
} else {
hi = mid - 1;
}
}

// After searching, lo is 1 greater than the
// length of the longest prefix of X[i]
auto newLength = lo;

// The predecessor of X[i] is the last index of
// the subsequence of length newLength-1
prevIndexes[i] = indexes[newLength - 1];
indexes[newLength] = i;

if (newLength > length) {
// If we found a subsequence longer than any we've
// found yet, update L
length = newLength;
}
}

// Reconstruct the longest increasing indexes
NSMutableIndexSet *result = [NSMutableIndexSet new];
auto k = indexes[length];
for (NSUInteger i = 0; i < length; i++) {
NSUInteger index = newResultsArray[k].index;

// Ignore inserted entries
if (index != NSNotFound) {
[result addIndex:index];
}
k = prevIndexes[k];
}
return result;
}

static BOOL mapContainsMoves(unordered_map<NSUInteger, NSUInteger> movesMap, NSUInteger fromIndex, NSUInteger toIndex)
{
auto iter = movesMap.find(fromIndex);
return iter != movesMap.end() && iter->second == toIndex;
}

static id IGListDiffing(BOOL returnIndexPaths,
NSInteger fromSection,
NSInteger toSection,
Expand Down Expand Up @@ -262,9 +321,17 @@ static id IGListDiffing(BOOL returnIndexPaths,
addIndexToMap(returnIndexPaths, fromSection, i, oldArray[i], oldMap);
}

// Calculate longest increasing indexes, identifying entries which do not require any moves
auto longestIncreasingSequence = longestIncreasingIndexes(newResultsArray);

// track offsets for previously made moves to identify unnecessary moves
NSInteger movesOffset = 0;

// reset and track offsets from inserted items to calculate where items have moved
runningOffset = 0;

unordered_map<NSUInteger, NSUInteger> moves;

for (NSInteger i = 0; i < newCount; i++) {
insertOffsets[i] = runningOffset;
const IGListRecord record = newResultsArray[i];
Expand All @@ -280,10 +347,20 @@ static id IGListDiffing(BOOL returnIndexPaths,
}

// calculate the offset and determine if there was a move
// if the indexes match, ignore the index
const NSInteger insertOffset = insertOffsets[i];
const NSInteger deleteOffset = deleteOffsets[oldIndex];
if ((oldIndex - deleteOffset + insertOffset) != i) {

auto oldIndexNoDeletes = oldIndex - deleteOffset;
auto oldCorrectedIndex = oldIndexNoDeletes + insertOffset;

// 1. if the indexes match, ignore the index, else
// 2. if the index requires no move and is not swap move, ignore the index, else
// 3. if the index matches old index with moves offset, ignore the index, else
if (oldCorrectedIndex != i
&& (![longestIncreasingSequence containsIndex:oldIndex] || mapContainsMoves(moves, i + deleteOffset - insertOffset, oldIndexNoDeletes))
&& ((oldCorrectedIndex + movesOffset) != i)) {

// add move from old index to new index
id move;
if (returnIndexPaths) {
NSIndexPath *from = [NSIndexPath indexPathForItem:oldIndex inSection:fromSection];
Expand All @@ -293,6 +370,12 @@ static id IGListDiffing(BOOL returnIndexPaths,
move = [[IGListMoveIndex alloc] initWithFrom:oldIndex to:i];
}
[mMoves addObject:move];

// store move for later checks
moves[oldIndex] = i;

// track offset caused by the move
movesOffset++;
}
}

Expand Down
194 changes: 182 additions & 12 deletions Tests/IGListDiffTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,31 @@
return [arr sortedArrayUsingSelector:@selector(compare:)];
}

static NSArray *updatedArray(NSArray *oldArray, NSArray *newArray, IGListIndexSetResult *diff) {
NSMutableArray *result = [oldArray mutableCopy];

NSMutableIndexSet *deletes = [diff.deletes mutableCopy];
NSMutableIndexSet *inserts = [diff.inserts mutableCopy];

NSMutableIndexSet *fromIndexes = [NSMutableIndexSet new];
NSMutableIndexSet *toIndexes = [NSMutableIndexSet new];

for (IGListMoveIndex *move in diff.moves) {
[fromIndexes addIndex:move.from];
[toIndexes addIndex:move.to];
}

[deletes addIndexes:fromIndexes];
[inserts addIndexes:toIndexes];

[result removeObjectsAtIndexes:deletes];

NSArray *insertedObjects = [newArray objectsAtIndexes:inserts];
[result insertObjects:insertedObjects atIndexes:inserts];

return result;
}

@implementation IGListDiffTests

- (void)test_whenDiffingEmptyArrays_thatResultHasNoChanges {
Expand All @@ -65,26 +90,26 @@ - (void)test_whenDiffingToEmptyArray_thatResultHasChanges {
XCTAssertEqual([result changeCount], 1);
}

- (void)test_whenSwappingObjects_thatResultHasMoves {
- (void)test_whenSwappingNeighbors_thatResultHasSingleMove {
NSArray *o = @[@1, @2];
NSArray *n = @[@2, @1];
IGListIndexSetResult *result = IGListDiff(o, n, IGListDiffEquality);
NSArray *expected = @[
[[IGListMoveIndex alloc] initWithFrom:0 to:1],
[[IGListMoveIndex alloc] initWithFrom:1 to:0],
[[IGListMoveIndex alloc] initWithFrom:1 to:0]
];
NSArray<IGListMoveIndexPath *> *sortedMoves = sorted(result.moves);
XCTAssertEqualObjects(sortedMoves, expected);
XCTAssertEqual([result changeCount], 2);
XCTAssertEqualObjects(result.moves, expected);
XCTAssertEqual([result changeCount], 1);
}

- (void)test_whenMovingObjectsTogether_thatResultHasMoves {
// "trick" is having multiple @3s
NSArray *o = @[@1, @2, @3, @3, @4];
NSArray *n = @[@2, @3, @1, @3, @4];
IGListIndexSetResult *result = IGListDiff(o, n, IGListDiffEquality);
IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:1 to:0]);
IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:0 to:2]);
// IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:1 to:0]);
// IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:0 to:2]);

XCTAssertEqualObjects(updatedArray(o, n, result), n);
}

- (void)test_whenDiffingWordsFromPaper_withIndexPaths_thatDeletesMatchPaper {
Expand Down Expand Up @@ -115,10 +140,8 @@ - (void)test_whenSwappingObjects_withIndexPaths_thatResultHasMoves {
IGListIndexPathResult *result = IGListDiffPaths(0, 0, o, n, IGListDiffEquality);
NSArray *expected = @[
[[IGListMoveIndexPath alloc] initWithFrom:genIndexPath(2, 0) to:genIndexPath(3, 0)],
[[IGListMoveIndexPath alloc] initWithFrom:genIndexPath(3, 0) to:genIndexPath(1, 0)],
];
NSArray<IGListMoveIndexPath *> *sortedMoves = sorted(result.moves);
XCTAssertEqualObjects(sortedMoves, expected);
XCTAssertEqualObjects(result.moves, expected);
}

- (void)test_whenObjectEqualityChanges_thatResultHasUpdates {
Expand Down Expand Up @@ -279,7 +302,7 @@ - (void)test_whenMovingObjectShiftsOthers_thatMovesContainRequiredMoves {
NSArray *n = @[@1, @4, @5, @2, @3, @6, @7];
IGListIndexSetResult *result = IGListDiff(o, n, IGListDiffEquality);
IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:3 to:1]);
IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:1 to:3]);
IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:4 to:2]);
}

- (void)test_whenDiffing_thatOldIndexesMatch {
Expand Down Expand Up @@ -404,4 +427,151 @@ - (void)test_whenDiffing_withBatchUpdateResult_thatIndexPathsMatch {
XCTAssertEqualObjects(sorted(result.inserts), expectedInserts);
}

- (void)test_whenMovingBackward_thatIndexesMatch {
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
NSArray *n = @[ @5, @4, @0, @1, @2, @3 ];
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
XCTAssertEqual(result.updates.count, 0);
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:5 to:0],
[[IGListMoveIndex alloc] initWithFrom:4 to:1] ];
XCTAssertEqualObjects(result.moves, expectedMoves);
NSMutableIndexSet *expectedDeletes = [NSMutableIndexSet new];
XCTAssertEqualObjects(result.deletes, expectedDeletes);
NSMutableIndexSet *expectedInserts = [NSMutableIndexSet new];
XCTAssertEqualObjects(result.inserts, expectedInserts);
}

- (void)test_whenMovingBackwardWithInsertionBefore_thatIndexesMatch {
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
NSArray *n = @[ @100, @5, @0, @1, @2, @3, @4 ];
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
XCTAssertEqual(result.updates.count, 0);
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:5 to:1] ];
XCTAssertEqualObjects(result.moves, expectedMoves);
NSMutableIndexSet *expectedDeletes = [NSMutableIndexSet new];
XCTAssertEqualObjects(result.deletes, expectedDeletes);
NSIndexSet *expectedInserts = [NSIndexSet indexSetWithIndex:0];
XCTAssertEqualObjects(result.inserts, expectedInserts);
}

- (void)test_whenMovingBackwardWithInsertionInBetween_thatIndexesMatch {
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
NSArray *n = @[ @5, @100, @0, @1, @2, @3, @4 ];
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
XCTAssertEqual(result.updates.count, 0);
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:5 to:0] ];
XCTAssertEqualObjects(result.moves, expectedMoves);
NSMutableIndexSet *expectedDeletes = [NSMutableIndexSet new];
XCTAssertEqualObjects(result.deletes, expectedDeletes);
NSIndexSet *expectedInserts = [NSIndexSet indexSetWithIndex:1];
XCTAssertEqualObjects(result.inserts, expectedInserts);
}

- (void)test_whenMovingBackwardWithDeletionBefore_thatIndexesMatch {
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
NSArray *n = @[ @1, @4, @5, @2, @3 ];
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
XCTAssertEqual(result.updates.count, 0);
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:4 to:1],
[[IGListMoveIndex alloc] initWithFrom:5 to:2] ];
XCTAssertEqualObjects(result.moves, expectedMoves);
NSIndexSet *expectedDeletes = [NSIndexSet indexSetWithIndex:0];
XCTAssertEqualObjects(result.deletes, expectedDeletes);
NSIndexSet *expectedInserts = [NSIndexSet new];
XCTAssertEqualObjects(result.inserts, expectedInserts);
}

- (void)test_whenMovingBackwardWithDeletionInBetween_thatIndexesMatch {
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
NSArray *n = @[ @5, @0, @2, @3, @4 ];
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
XCTAssertEqual(result.updates.count, 0);
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:5 to:0] ];
XCTAssertEqualObjects(result.moves, expectedMoves);
NSIndexSet *expectedDeletes = [NSIndexSet indexSetWithIndex:1];
XCTAssertEqualObjects(result.deletes, expectedDeletes);
NSIndexSet *expectedInserts = [NSIndexSet new];
XCTAssertEqualObjects(result.inserts, expectedInserts);
}

- (void)test_whenMovingForward_thatIndexesMatch {
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
NSArray *n = @[ @2, @3, @4, @5, @1, @0 ];
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
XCTAssertEqual(result.updates.count, 0);
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:1 to:4],
[[IGListMoveIndex alloc] initWithFrom:0 to:5] ];
XCTAssertEqualObjects(result.moves, expectedMoves);
NSMutableIndexSet *expectedDeletes = [NSMutableIndexSet new];
XCTAssertEqualObjects(result.deletes, expectedDeletes);
NSMutableIndexSet *expectedInserts = [NSMutableIndexSet new];
XCTAssertEqualObjects(result.inserts, expectedInserts);
}

- (void)test_whenMovingBothWays_thatIndexesMatch {
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
NSArray *n = @[ @1, @2, @0, @5, @3, @4 ];
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
XCTAssertEqual(result.updates.count, 0);
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:0 to:2],
[[IGListMoveIndex alloc] initWithFrom:5 to:3] ];
XCTAssertEqualObjects(result.moves, expectedMoves);
NSMutableIndexSet *expectedDeletes = [NSMutableIndexSet new];
XCTAssertEqualObjects(result.deletes, expectedDeletes);
NSMutableIndexSet *expectedInserts = [NSMutableIndexSet new];
XCTAssertEqualObjects(result.inserts, expectedInserts);
}

- (void)test_whenRandomlyUpdating_thatApplyingDiffProducesSameArray {
NSArray *o = @[ @0, @1, @2, @3, @4, @5, @6, @7, @8, @9 ];

for (int testIdx = 0; testIdx < 1000; testIdx ++) {

NSMutableArray *n = [o mutableCopy];

for (int i = 0; i < 2; i++) {
[n removeObjectAtIndex:arc4random_uniform((uint32_t) n.count)];
}

NSMutableIndexSet *fromIndexes = [NSMutableIndexSet new];
NSMutableIndexSet *toIndexes = [NSMutableIndexSet new];

for (int i = 0; i < 2; i++) {
NSUInteger fromIdx;
do {
fromIdx = arc4random_uniform((uint32_t) n.count);
} while ([fromIndexes containsIndex:fromIdx] || [toIndexes containsIndex:fromIdx]);
[fromIndexes addIndex:fromIdx];

NSUInteger toIdx;
do {
toIdx = arc4random_uniform((uint32_t) n.count);
} while ([toIndexes containsIndex:fromIdx] || [toIndexes containsIndex:toIdx]);
[toIndexes addIndex:toIdx];
}

NSArray *movedObjects = [n objectsAtIndexes:fromIndexes];
[n removeObjectsAtIndexes:fromIndexes];
[n insertObjects:movedObjects atIndexes:toIndexes];

NSMutableIndexSet *inserts = [NSMutableIndexSet new];
NSMutableArray *insertedObjects = [NSMutableArray new];

for (int i = 0; i < 3; i++) {
NSUInteger idx;
do {
idx = arc4random_uniform((uint32_t) n.count);
} while ([inserts containsIndex:idx]);
[inserts addIndex:idx];
[insertedObjects addObject:@(n.count + i + 10)];
}

[n insertObjects:insertedObjects atIndexes:inserts];

IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
XCTAssertEqualObjects(updatedArray(o, n, result), n);
}
}

@end

0 comments on commit 96f1a43

Please sign in to comment.