Added modifiableForEach extension method that allows for in place ListModification during traversal - #768
Added modifiableForEach extension method that allows for in place ListModification during traversal#768tunjid wants to merge 3 commits into
Conversation
|
Over to Nick since he's been working on the Animator. |
nickbutcher
left a comment
nickbutcher
left a comment
There was a problem hiding this comment.
Thanks for the contribution! Added a few review comments.
| * A for each loop where traversal is backed by an [Iterator] allowing for list modification | ||
| * in place. | ||
| */ | ||
| inline fun <T> Iterable<T>.modifiableForEach(action: (T) -> Unit) { |
There was a problem hiding this comment.
Not sure about the name here, to me 'modifiable', implies that this modifies each element, not that the underling collection can be modified.
| * in place. | ||
| */ | ||
| inline fun <T> Iterable<T>.modifiableForEach(action: (T) -> Unit) { | ||
| iterator().apply { |
There was a problem hiding this comment.
The apply block feels less idiomatic here. Why not just val iterator = iterator() like most of the stdlib does?
| */ | ||
| inline fun <T> Iterable<T>.modifiableForEach(action: (T) -> Unit) { | ||
| iterator().apply { | ||
| while (hasNext()) next().apply(action) |
There was a problem hiding this comment.
again the apply feels unnecessary, why not just while (iterator.hasNext()) action(next())
| iterator().apply { | ||
| while (hasNext()) next().apply(action) | ||
| } | ||
| } No newline at end of file |
|
@nickbutcher thanks for the comments! Are rebases preferred to merges when there are changes upstream? I checked the contributing doc, but it doesn't seem to mention it. |
|
@tunjid rebase please. Thanks! |
…tModification during traversal
d4a10c3 to
9dc1a51
Compare
|
I'm not sure of a better name than |
|
Hmm, I can't think of a great name either and worry that |
|
That works! What do you think about adding an iterator prefix so the mutable iterator backing is communicated? So |
4 participants
📢 Type of change
📜 Description
Added modifiableForEach extension method that allows for in place List modification during traversal.
💡 Motivation and Context
Adresses #767
If the SlideInItemAnimator is running during an orientation change, a
ConcurrentModificationExceptionwill be thrown. This change allows for traversal with an iterator, allowing for in place modifications to the list.💚 How did you test it?
I used the animator in another project where the SlideInItemAnimator runs continuously, and triggered Activity destruction by rotating the phone. Source for that class is here: https://github.com/tunjid/android-bootstrap/blob/develop/app/src/main/java/com/tunjid/androidbootstrap/fragments/ShiftingTileFragment.kt
📝 Checklist
./gradlew spotlessApplybefore submitting the PR🔮 Next steps
📸 Screenshots / GIFs