Instagram / Instagram/IGListKit
[RFC] Change IGListDiff result type to generic object with array of operations
- Dominant language
- Objective-C
- Stars
- 13.1k
- Forks
- 1.5k
- PR merge metrics
- No merged PRs in 30d
Description
Had a shower-thought this morning: what if we change the result produced by `IGListDiff(...)` to be a generic result object with an array of operations (delete/insert/reload/move)?
### Why?
There are currently too many things doing essentially the same stuff:
- [IGListIndexSetResult](https://github.com/Instagram/IGListKit/blob/master/Source/Common/IGListIndexSetResult.h) and [IGListIndexPathResult](https://github.com/Instagram/IGListKit/blob/master/Source/Common/IGListIndexPathResult.h)
- `NSIndexSet` or `NSArray`
- Have multiple delete, insert, update, and moves
- Keep a map tracking from/to for identifiers
- [IGListMoveIndex](https://github.com/Instagram/IGListKit/blob/master/Source/Common/IGListMoveIndex.h) and [IGListMoveIndexPath](https://github.com/Instagram/IGListKit/blob/master/Source/Common/IGListMoveIndexPath.h)
- `NSInteger` or `NSIndexPath`
- Has a to/from
- [IGListReloadIndexPath](https://github.com/Instagram/IGListKit/blob/master/Source/Internal/IGListReloadIndexPath.h)
- `NSIndexPath`
- Has a to/from
There's a lot of duplication going on here. I think we can clean it up:
### Proposal
Create a generic "operation" object:
```objc
@interface IGListOperation: NSObject
@property (nonatomic, strong, readonly) ObjectType from;
@property (nonatomic, strong, readonly) ObjectType to;
@property (nonatomic, assign, readonly) IGListOperationType type; // delete, insert, move, update
@end
```
Then we create a new result object:
```objc
@interface IGListDiffResult: NSObject
@property (nonatomic, strong, readonly) NSArray *> *operations;
@end
```
I'd also love to restore diff statistics (like clocking the time it takes to diff). We used to collect this data.
We could also add back APIs like `-[IGListDiffResult inserts]` that just query an internal map of operation types to operations. Might be convenient?
### Considerations
- We [merge diff results](https://github.com/Instagram/IGListKit/blob/master/Source/IGListAdapterUpdater.m#L254-L302) with other coalesced operations. This still has to happen.
- Operations should implement `isEqual:` and `hash` so we can add and unique them in `NSSet`s
### Questions
- Can we intermix `NSIndexPath` and `NSInteger` with generic types?
- I don't think so, and if even if we could, I don't think we could right a proper `hash` and `isEqual` for the operation
- Is unpacking `NSNumber` with `integerValue` when enumerating operations a concern at all? The perf overhead should be minuscule, but it does exist
Contributor guide
Assessment
This issue has not been assessed yet.