From d8a728ec7f40998f8b5a57537d34ebd997f94d71 Mon Sep 17 00:00:00 2001 From: Maxime Ollivier Date: Tue, 8 Sep 2020 09:06:16 -0700 Subject: [PATCH] clean up IGListReloadTransaction Summary: Using blocks can be nice, but here it causes a couple problems: * The order of execution is a bit hard to follow. For example, in `-begin`, the very first thing we see defined is `executeCompletionBlocks` which is the last thing actually called. `IGListReloadTransaction` isn't too complex, but it gets even harder to follow in `IGListBatchUpdateTransaction`. * It makes crash and assert reports hard to understand, because the stack includes mostly block names. So lets turn blocks into methods. Reviewed By: patters Differential Revision: D23151918 fbshipit-source-id: 3ac4c77e9978c2700fd6744f084f624d27f0d300 --- .../Internal/IGListReloadTransaction.m | 80 +++++++++---------- 1 file changed, 36 insertions(+), 44 deletions(-) diff --git a/Source/IGListKit/Internal/IGListReloadTransaction.m b/Source/IGListKit/Internal/IGListReloadTransaction.m index 8618292e..950a3b5c 100644 --- a/Source/IGListKit/Internal/IGListReloadTransaction.m +++ b/Source/IGListKit/Internal/IGListReloadTransaction.m @@ -11,8 +11,8 @@ @interface IGListReloadTransaction () // Given -@property (nonatomic, copy, readonly) IGListCollectionViewBlock collectionViewBlock; -@property (nonatomic, weak, readonly, nullable) id updater; +@property (nonatomic, copy, readonly) UICollectionView *collectionView; +@property (nonatomic, weak, readonly) id updater; @property (nonatomic, weak, readonly, nullable) id delegate; @property (nonatomic, copy, readonly) IGListReloadUpdateBlock reloadBlock; @property (nonatomic, copy, readonly) NSArray *itemUpdateBlocks; @@ -26,12 +26,12 @@ - (instancetype)initWithCollectionViewBlock:(IGListCollectionViewBlock)collectionViewBlock updater:(id)updater - delegate:(nullable id)delegate + delegate:(id)delegate reloadBlock:(IGListReloadUpdateBlock)reloadBlock itemUpdateBlocks:(NSArray *)itemUpdateBlocks completionBlocks:(NSArray *)completionBlocks { if (self = [super init]) { - _collectionViewBlock = [collectionViewBlock copy]; + _collectionView = collectionViewBlock ? collectionViewBlock() : nil; _updater = updater; _delegate = delegate; _reloadBlock = [reloadBlock copy]; @@ -43,69 +43,54 @@ return self; } -#pragma mark - IGListUpdateTransactable +#pragma mark - Update - (void)begin { - IGListCollectionViewBlock collectionViewBlock = self.collectionViewBlock; - id updater = self.updater; - id delegate = self.delegate; - void (^reloadUpdates)(void) = self.reloadBlock; - NSArray *itemUpdateBlocks = self.itemUpdateBlocks; - NSArray *completionBlocks = self.completionBlocks; - - void (^executeCompletionBlocks)(BOOL) = ^(BOOL finished) { - for (IGListUpdatingCompletion block in completionBlocks) { - block(finished); - } - - // Execute any completion blocks from item updates. Added after item blocks are executed in order to capture any - // re-entrant updates. - NSArray *inUpdateCompletionBlocks = [self.inUpdateCompletionBlocks copy]; - for (IGListUpdatingCompletion block in inUpdateCompletionBlocks) { - block(finished); - } - - self.state = IGListBatchUpdateStateIdle; - }; - // bail early if the collection view has been deallocated in the time since the update was queued - UICollectionView *collectionView = collectionViewBlock(); - if (collectionView == nil) { - executeCompletionBlocks(NO); - [delegate listAdapterUpdater:updater didFinishWithoutUpdatesWithCollectionView:collectionView]; + if (self.collectionView == nil) { + [self.delegate listAdapterUpdater:self.updater didFinishWithoutUpdatesWithCollectionView:self.collectionView]; + [self _executeCompletionBlocks:YES]; return; } // item updates must not send mutations to the collection view while we are reloading self.state = IGListBatchUpdateStateExecutingBatchUpdateBlock; - if (reloadUpdates) { - reloadUpdates(); + if (self.reloadBlock) { + self.reloadBlock(); } // execute all stored item update blocks even if we are just calling reloadData. the actual collection view // mutations will be discarded, but clients are encouraged to put their actual /data/ mutations inside the // update block as well, so if we don't execute the block the changes will never happen - for (IGListItemUpdateBlock itemUpdateBlock in itemUpdateBlocks) { + for (IGListItemUpdateBlock itemUpdateBlock in self.itemUpdateBlocks) { itemUpdateBlock(); } self.state = IGListBatchUpdateStateExecutedBatchUpdateBlock; - [delegate listAdapterUpdater:updater willReloadDataWithCollectionView:collectionView isFallbackReload:NO]; - [collectionView reloadData]; - [collectionView.collectionViewLayout invalidateLayout]; - [collectionView layoutIfNeeded]; - [delegate listAdapterUpdater:updater didReloadDataWithCollectionView:collectionView isFallbackReload:NO]; + [self.delegate listAdapterUpdater:self.updater willReloadDataWithCollectionView:self.collectionView isFallbackReload:NO]; + [self.collectionView reloadData]; + [self.collectionView.collectionViewLayout invalidateLayout]; + [self.collectionView layoutIfNeeded]; + [self.delegate listAdapterUpdater:self.updater didReloadDataWithCollectionView:self.collectionView isFallbackReload:NO]; - executeCompletionBlocks(YES); + [self _executeCompletionBlocks:YES]; } -- (void)addCompletionBlock:(IGListUpdatingCompletion)completion { - if (!self.inUpdateCompletionBlocks) { - _inUpdateCompletionBlocks = [NSMutableArray new]; +- (void)_executeCompletionBlocks:(BOOL)finished { + for (IGListUpdatingCompletion block in self.completionBlocks) { + block(finished); } - [self.inUpdateCompletionBlocks addObject:completion]; + + // Execute any completion blocks from item updates. Added after item blocks are executed in order to capture any + // re-entrant updates. + NSArray *inUpdateCompletionBlocks = [self.inUpdateCompletionBlocks copy]; + for (IGListUpdatingCompletion block in inUpdateCompletionBlocks) { + block(finished); + } + + self.state = IGListBatchUpdateStateIdle; } #pragma mark - Item updates @@ -130,4 +115,11 @@ // no-op. Reloading all cells. } +- (void)addCompletionBlock:(IGListUpdatingCompletion)completion { + if (!self.inUpdateCompletionBlocks) { + _inUpdateCompletionBlocks = [NSMutableArray new]; + } + [self.inUpdateCompletionBlocks addObject:completion]; +} + @end