Skip to content

Commit 56a1ced

Browse files
author
Nickolay Tarbayev
committed
Fixes moves calculation to produce only minimal required moves
1 parent df18195 commit 56a1ced

File tree

2 files changed

+268
-15
lines changed

2 files changed

+268
-15
lines changed

Source/Common/IGListDiff.mm

Lines changed: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,65 @@ static void addIndexToCollection(BOOL useIndexPaths, __unsafe_unretained id coll
9494
return paths;
9595
}
9696

97+
// Calculates longest increasing indexes set using O(n log n) complexity algorithm
98+
static NSIndexSet *longestIncreasingIndexes(vector<IGListRecord> newResultsArray)
99+
{
100+
NSUInteger count = newResultsArray.size();
101+
vector<NSUInteger> prevIndexes(count);
102+
vector<NSUInteger> indexes(count + 1);
103+
104+
NSUInteger length = 0;
105+
for (NSUInteger i = 0; i < count; i++) {
106+
// Binary search for the largest positive j ≤ length
107+
// such that X[M[j]] < X[i]
108+
NSUInteger lo = 1;
109+
NSUInteger hi = length;
110+
while (lo <= hi) {
111+
auto mid = ceil((lo + hi) / 2);
112+
if (newResultsArray[indexes[mid]].index < newResultsArray[i].index) {
113+
lo = mid + 1;
114+
} else {
115+
hi = mid - 1;
116+
}
117+
}
118+
119+
// After searching, lo is 1 greater than the
120+
// length of the longest prefix of X[i]
121+
auto newLength = lo;
122+
123+
// The predecessor of X[i] is the last index of
124+
// the subsequence of length newLength-1
125+
prevIndexes[i] = indexes[newLength - 1];
126+
indexes[newLength] = i;
127+
128+
if (newLength > length) {
129+
// If we found a subsequence longer than any we've
130+
// found yet, update L
131+
length = newLength;
132+
}
133+
}
134+
135+
// Reconstruct the longest increasing indexes
136+
NSMutableIndexSet *result = [NSMutableIndexSet new];
137+
auto k = indexes[length];
138+
for (NSUInteger i = 0; i < length; i++) {
139+
NSUInteger index = newResultsArray[k].index;
140+
141+
// Ignore inserted entries
142+
if (index != NSNotFound) {
143+
[result addIndex:index];
144+
}
145+
k = prevIndexes[k];
146+
}
147+
return result;
148+
}
149+
150+
static BOOL mapContainsMoves(unordered_map<NSUInteger, NSUInteger> movesMap, NSUInteger fromIndex, NSUInteger toIndex)
151+
{
152+
auto iter = movesMap.find(fromIndex);
153+
return iter != movesMap.end() && iter->second == toIndex;
154+
}
155+
97156
static id IGListDiffing(BOOL returnIndexPaths,
98157
NSInteger fromSection,
99158
NSInteger toSection,
@@ -262,9 +321,17 @@ static id IGListDiffing(BOOL returnIndexPaths,
262321
addIndexToMap(returnIndexPaths, fromSection, i, oldArray[i], oldMap);
263322
}
264323

324+
// Calculate longest increasing indexes, identifying entries which do not require any moves
325+
auto longestIncreasingSequence = longestIncreasingIndexes(newResultsArray);
326+
327+
// track offsets for previously made moves to identify unnecessary moves
328+
NSInteger movesOffset = 0;
329+
265330
// reset and track offsets from inserted items to calculate where items have moved
266331
runningOffset = 0;
267332

333+
unordered_map<NSUInteger, NSUInteger> moves;
334+
268335
for (NSInteger i = 0; i < newCount; i++) {
269336
insertOffsets[i] = runningOffset;
270337
const IGListRecord record = newResultsArray[i];
@@ -280,10 +347,20 @@ static id IGListDiffing(BOOL returnIndexPaths,
280347
}
281348

282349
// calculate the offset and determine if there was a move
283-
// if the indexes match, ignore the index
284350
const NSInteger insertOffset = insertOffsets[i];
285351
const NSInteger deleteOffset = deleteOffsets[oldIndex];
286-
if ((oldIndex - deleteOffset + insertOffset) != i) {
352+
353+
auto oldIndexNoDeletes = oldIndex - deleteOffset;
354+
auto oldCorrectedIndex = oldIndexNoDeletes + insertOffset;
355+
356+
// 1. if the indexes match, ignore the index, else
357+
// 2. if the index requires no move and is not swap move, ignore the index, else
358+
// 3. if the index matches old index with moves offset, ignore the index, else
359+
if (oldCorrectedIndex != i
360+
&& (![longestIncreasingSequence containsIndex:oldIndex] || mapContainsMoves(moves, i + deleteOffset - insertOffset, oldIndexNoDeletes))
361+
&& ((oldCorrectedIndex + movesOffset) != i)) {
362+
363+
// add move from old index to new index
287364
id move;
288365
if (returnIndexPaths) {
289366
NSIndexPath *from = [NSIndexPath indexPathForItem:oldIndex inSection:fromSection];
@@ -293,6 +370,12 @@ static id IGListDiffing(BOOL returnIndexPaths,
293370
move = [[IGListMoveIndex alloc] initWithFrom:oldIndex to:i];
294371
}
295372
[mMoves addObject:move];
373+
374+
// store move for later checks
375+
moves[oldIndex] = i;
376+
377+
// track offset caused by the move
378+
movesOffset++;
296379
}
297380
}
298381

Tests/IGListDiffTests.m

Lines changed: 183 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* All rights reserved.
44
*
55
* This source code is licensed under the BSD-style license found in the
6-
* LICENSE file in the root directory of this source tree. An additional grant
6+
* LICENSE file in the root directory of this source tree. An additional grant
77
* of patent rights can be found in the PATENTS file in the same directory.
88
*/
99

@@ -40,6 +40,31 @@
4040
return [arr sortedArrayUsingSelector:@selector(compare:)];
4141
}
4242

43+
static NSArray *updatedArray(NSArray *oldArray, NSArray *newArray, IGListIndexSetResult *diff) {
44+
NSMutableArray *result = [oldArray mutableCopy];
45+
46+
NSMutableIndexSet *deletes = [diff.deletes mutableCopy];
47+
NSMutableIndexSet *inserts = [diff.inserts mutableCopy];
48+
49+
NSMutableIndexSet *fromIndexes = [NSMutableIndexSet new];
50+
NSMutableIndexSet *toIndexes = [NSMutableIndexSet new];
51+
52+
for (IGListMoveIndex *move in diff.moves) {
53+
[fromIndexes addIndex:move.from];
54+
[toIndexes addIndex:move.to];
55+
}
56+
57+
[deletes addIndexes:fromIndexes];
58+
[inserts addIndexes:toIndexes];
59+
60+
[result removeObjectsAtIndexes:deletes];
61+
62+
NSArray *insertedObjects = [newArray objectsAtIndexes:inserts];
63+
[result insertObjects:insertedObjects atIndexes:inserts];
64+
65+
return result;
66+
}
67+
4368
@implementation IGListDiffTests
4469

4570
- (void)test_whenDiffingEmptyArrays_thatResultHasNoChanges {
@@ -65,26 +90,26 @@ - (void)test_whenDiffingToEmptyArray_thatResultHasChanges {
6590
XCTAssertEqual([result changeCount], 1);
6691
}
6792

68-
- (void)test_whenSwappingObjects_thatResultHasMoves {
93+
- (void)test_whenSwappingNeighbors_thatResultHasSingleMove {
6994
NSArray *o = @[@1, @2];
7095
NSArray *n = @[@2, @1];
7196
IGListIndexSetResult *result = IGListDiff(o, n, IGListDiffEquality);
7297
NSArray *expected = @[
73-
[[IGListMoveIndex alloc] initWithFrom:0 to:1],
74-
[[IGListMoveIndex alloc] initWithFrom:1 to:0],
98+
[[IGListMoveIndex alloc] initWithFrom:1 to:0]
7599
];
76-
NSArray<IGListMoveIndexPath *> *sortedMoves = sorted(result.moves);
77-
XCTAssertEqualObjects(sortedMoves, expected);
78-
XCTAssertEqual([result changeCount], 2);
100+
XCTAssertEqualObjects(result.moves, expected);
101+
XCTAssertEqual([result changeCount], 1);
79102
}
80103

81104
- (void)test_whenMovingObjectsTogether_thatResultHasMoves {
82105
// "trick" is having multiple @3s
83106
NSArray *o = @[@1, @2, @3, @3, @4];
84107
NSArray *n = @[@2, @3, @1, @3, @4];
85108
IGListIndexSetResult *result = IGListDiff(o, n, IGListDiffEquality);
86-
IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:1 to:0]);
87-
IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:0 to:2]);
109+
// IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:1 to:0]);
110+
// IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:0 to:2]);
111+
112+
XCTAssertEqualObjects(updatedArray(o, n, result), n);
88113
}
89114

90115
- (void)test_whenDiffingWordsFromPaper_withIndexPaths_thatDeletesMatchPaper {
@@ -115,10 +140,8 @@ - (void)test_whenSwappingObjects_withIndexPaths_thatResultHasMoves {
115140
IGListIndexPathResult *result = IGListDiffPaths(0, 0, o, n, IGListDiffEquality);
116141
NSArray *expected = @[
117142
[[IGListMoveIndexPath alloc] initWithFrom:genIndexPath(2, 0) to:genIndexPath(3, 0)],
118-
[[IGListMoveIndexPath alloc] initWithFrom:genIndexPath(3, 0) to:genIndexPath(1, 0)],
119143
];
120-
NSArray<IGListMoveIndexPath *> *sortedMoves = sorted(result.moves);
121-
XCTAssertEqualObjects(sortedMoves, expected);
144+
XCTAssertEqualObjects(result.moves, expected);
122145
}
123146

124147
- (void)test_whenObjectEqualityChanges_thatResultHasUpdates {
@@ -279,7 +302,7 @@ - (void)test_whenMovingObjectShiftsOthers_thatMovesContainRequiredMoves {
279302
NSArray *n = @[@1, @4, @5, @2, @3, @6, @7];
280303
IGListIndexSetResult *result = IGListDiff(o, n, IGListDiffEquality);
281304
IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:3 to:1]);
282-
IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:1 to:3]);
305+
IGAssertContains(result.moves, [[IGListMoveIndex alloc] initWithFrom:4 to:2]);
283306
}
284307

285308
- (void)test_whenDiffing_thatOldIndexesMatch {
@@ -404,4 +427,151 @@ - (void)test_whenDiffing_withBatchUpdateResult_thatIndexPathsMatch {
404427
XCTAssertEqualObjects(sorted(result.inserts), expectedInserts);
405428
}
406429

430+
- (void)test_whenMovingBackward_thatIndexesMatch {
431+
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
432+
NSArray *n = @[ @5, @4, @0, @1, @2, @3 ];
433+
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
434+
XCTAssertEqual(result.updates.count, 0);
435+
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:5 to:0],
436+
[[IGListMoveIndex alloc] initWithFrom:4 to:1] ];
437+
XCTAssertEqualObjects(result.moves, expectedMoves);
438+
NSMutableIndexSet *expectedDeletes = [NSMutableIndexSet new];
439+
XCTAssertEqualObjects(result.deletes, expectedDeletes);
440+
NSMutableIndexSet *expectedInserts = [NSMutableIndexSet new];
441+
XCTAssertEqualObjects(result.inserts, expectedInserts);
442+
}
443+
444+
- (void)test_whenMovingBackwardWithInsertionBefore_thatIndexesMatch {
445+
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
446+
NSArray *n = @[ @100, @5, @0, @1, @2, @3, @4 ];
447+
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
448+
XCTAssertEqual(result.updates.count, 0);
449+
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:5 to:1] ];
450+
XCTAssertEqualObjects(result.moves, expectedMoves);
451+
NSMutableIndexSet *expectedDeletes = [NSMutableIndexSet new];
452+
XCTAssertEqualObjects(result.deletes, expectedDeletes);
453+
NSIndexSet *expectedInserts = [NSIndexSet indexSetWithIndex:0];
454+
XCTAssertEqualObjects(result.inserts, expectedInserts);
455+
}
456+
457+
- (void)test_whenMovingBackwardWithInsertionInBetween_thatIndexesMatch {
458+
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
459+
NSArray *n = @[ @5, @100, @0, @1, @2, @3, @4 ];
460+
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
461+
XCTAssertEqual(result.updates.count, 0);
462+
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:5 to:0] ];
463+
XCTAssertEqualObjects(result.moves, expectedMoves);
464+
NSMutableIndexSet *expectedDeletes = [NSMutableIndexSet new];
465+
XCTAssertEqualObjects(result.deletes, expectedDeletes);
466+
NSIndexSet *expectedInserts = [NSIndexSet indexSetWithIndex:1];
467+
XCTAssertEqualObjects(result.inserts, expectedInserts);
468+
}
469+
470+
- (void)test_whenMovingBackwardWithDeletionBefore_thatIndexesMatch {
471+
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
472+
NSArray *n = @[ @1, @4, @5, @2, @3 ];
473+
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
474+
XCTAssertEqual(result.updates.count, 0);
475+
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:4 to:1],
476+
[[IGListMoveIndex alloc] initWithFrom:5 to:2] ];
477+
XCTAssertEqualObjects(result.moves, expectedMoves);
478+
NSIndexSet *expectedDeletes = [NSIndexSet indexSetWithIndex:0];
479+
XCTAssertEqualObjects(result.deletes, expectedDeletes);
480+
NSIndexSet *expectedInserts = [NSIndexSet new];
481+
XCTAssertEqualObjects(result.inserts, expectedInserts);
482+
}
483+
484+
- (void)test_whenMovingBackwardWithDeletionInBetween_thatIndexesMatch {
485+
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
486+
NSArray *n = @[ @5, @0, @2, @3, @4 ];
487+
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
488+
XCTAssertEqual(result.updates.count, 0);
489+
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:5 to:0] ];
490+
XCTAssertEqualObjects(result.moves, expectedMoves);
491+
NSIndexSet *expectedDeletes = [NSIndexSet indexSetWithIndex:1];
492+
XCTAssertEqualObjects(result.deletes, expectedDeletes);
493+
NSIndexSet *expectedInserts = [NSIndexSet new];
494+
XCTAssertEqualObjects(result.inserts, expectedInserts);
495+
}
496+
497+
- (void)test_whenMovingForward_thatIndexesMatch {
498+
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
499+
NSArray *n = @[ @2, @3, @4, @5, @1, @0 ];
500+
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
501+
XCTAssertEqual(result.updates.count, 0);
502+
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:1 to:4],
503+
[[IGListMoveIndex alloc] initWithFrom:0 to:5] ];
504+
XCTAssertEqualObjects(result.moves, expectedMoves);
505+
NSMutableIndexSet *expectedDeletes = [NSMutableIndexSet new];
506+
XCTAssertEqualObjects(result.deletes, expectedDeletes);
507+
NSMutableIndexSet *expectedInserts = [NSMutableIndexSet new];
508+
XCTAssertEqualObjects(result.inserts, expectedInserts);
509+
}
510+
511+
- (void)test_whenMovingBothWays_thatIndexesMatch {
512+
NSArray *o = @[ @0, @1, @2, @3, @4, @5 ];
513+
NSArray *n = @[ @1, @2, @0, @5, @3, @4 ];
514+
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
515+
XCTAssertEqual(result.updates.count, 0);
516+
NSArray *expectedMoves = @[ [[IGListMoveIndex alloc] initWithFrom:0 to:2],
517+
[[IGListMoveIndex alloc] initWithFrom:5 to:3] ];
518+
XCTAssertEqualObjects(result.moves, expectedMoves);
519+
NSMutableIndexSet *expectedDeletes = [NSMutableIndexSet new];
520+
XCTAssertEqualObjects(result.deletes, expectedDeletes);
521+
NSMutableIndexSet *expectedInserts = [NSMutableIndexSet new];
522+
XCTAssertEqualObjects(result.inserts, expectedInserts);
523+
}
524+
525+
- (void)test_whenRandomlyUpdating_thatApplyingDiffProducesSameArray {
526+
NSArray *o = @[ @0, @1, @2, @3, @4, @5, @6, @7, @8, @9 ];
527+
528+
for (int testIdx = 0; testIdx < 1000; testIdx ++) {
529+
530+
NSMutableArray *n = [o mutableCopy];
531+
532+
for (int i = 0; i < 2; i++) {
533+
[n removeObjectAtIndex:arc4random_uniform((uint32_t) n.count)];
534+
}
535+
536+
NSMutableIndexSet *fromIndexes = [NSMutableIndexSet new];
537+
NSMutableIndexSet *toIndexes = [NSMutableIndexSet new];
538+
539+
for (int i = 0; i < 2; i++) {
540+
NSUInteger fromIdx;
541+
do {
542+
fromIdx = arc4random_uniform((uint32_t) n.count);
543+
} while ([fromIndexes containsIndex:fromIdx] || [toIndexes containsIndex:fromIdx]);
544+
[fromIndexes addIndex:fromIdx];
545+
546+
NSUInteger toIdx;
547+
do {
548+
toIdx = arc4random_uniform((uint32_t) n.count);
549+
} while ([toIndexes containsIndex:fromIdx] || [toIndexes containsIndex:toIdx]);
550+
[toIndexes addIndex:toIdx];
551+
}
552+
553+
NSArray *movedObjects = [n objectsAtIndexes:fromIndexes];
554+
[n removeObjectsAtIndexes:fromIndexes];
555+
[n insertObjects:movedObjects atIndexes:toIndexes];
556+
557+
NSMutableIndexSet *inserts = [NSMutableIndexSet new];
558+
NSMutableArray *insertedObjects = [NSMutableArray new];
559+
560+
for (int i = 0; i < 3; i++) {
561+
NSUInteger idx;
562+
do {
563+
idx = arc4random_uniform((uint32_t) n.count);
564+
} while ([inserts containsIndex:idx]);
565+
[inserts addIndex:idx];
566+
[insertedObjects addObject:@(n.count + i + 10)];
567+
}
568+
569+
[n insertObjects:insertedObjects atIndexes:inserts];
570+
571+
IGListIndexSetResult *result = [IGListDiff(o, n, IGListDiffEquality) resultForBatchUpdates];
572+
XCTAssertEqualObjects(updatedArray(o, n, result), n);
573+
}
574+
}
575+
407576
@end
577+

0 commit comments

Comments
 (0)