Fix failing duplicate identifier detection #375
Conversation
|
I've also added a fix for the bug. I don't need to create a separate issue for this right? |
teameh
changed the title
| throw Error.duplicateItem(item: item) | ||
| } | ||
| finalKeys.insert(key) | ||
|
|
||
| guard let initialItemPathIndex = dictionary[key] else { | ||
| continue |
There was a problem hiding this comment.
The problem here is that if the item was not present in the initial sections, we always continue and it's never checked if the finalSections contain items with duplicate identifiers!
| throw Error.duplicateSection(section: section) | ||
| } | ||
| finalSectionIdentities.insert(section.identity) | ||
|
|
||
| guard let initialSectionIndex = initialSectionIndexes[section.identity] else { | ||
| continue |
There was a problem hiding this comment.
Same here, we need to not only look in the initialSectionIndexes but also store the identity in an extra Set (any better data types for this use case?) and check if the final sections don't contain any duplicates
| ] | ||
|
|
||
| do { | ||
| _ = try Diff.differencesForSectionedView(initialSections: [], finalSections: final) |
There was a problem hiding this comment.
Without the fix this will not throw
| @@ -332,7 +401,7 @@ extension AlgorithmTests { | |||
| ] | |||
|
|
|||
| do { | |||
| _ = try Diff.differencesForSectionedView(initialSections: initial, finalSections: initial) | |||
| _ = try Diff.differencesForSectionedView(initialSections: [], finalSections: final) | |||
There was a problem hiding this comment.
Without the fix this will not throw
| @@ -357,7 +451,7 @@ extension AlgorithmTests { | |||
| ] | |||
|
|
|||
| do { | |||
| _ = try Diff.differencesForSectionedView(initialSections: initial, finalSections: initial) | |||
| _ = try Diff.differencesForSectionedView(initialSections: [], finalSections: final) | |||
There was a problem hiding this comment.
Without the fix this will not throw
|
@freak4pc I see you closed this? Is the issue fixed in 6.0.0? |
|
Can you reopen this against main? I changed the main branch name and apparently it closed all the PRs leading to it 🤦♂️🤦♂️🤦♂️ |
|
Haha maybe that's what happened at apple as well? 😜 here you go #390 |
Is not throwing
Duplicate itemorDuplicate sectionerror's whenfinalSectionscontains duplicates whileinitialSectionsdid not.I'll see if I can add the fix to this pull request as well.