Added StateKeeperOwner.withSavedState and InstanceKeeperOwner.retainedInstance extensions #171
Conversation
arkivanov
changed the title
WalkthroughThe recent update enhances the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- instance-keeper/api/instance-keeper.klib.api (1 hunks)
- instance-keeper/src/commonMain/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExt.kt (2 hunks)
- state-keeper/api/android/state-keeper.api (1 hunks)
- state-keeper/api/jvm/state-keeper.api (1 hunks)
- state-keeper/api/state-keeper.klib.api (1 hunks)
- state-keeper/src/commonMain/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExt.kt (1 hunks)
- state-keeper/src/commonTest/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExtTest.kt (1 hunks)
Additional comments not posted (12)
state-keeper/src/commonTest/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExtTest.kt (2)
9-33: Test method looks good! Consider adding more test cases.The
withSavedState_saves_and_restores_statetest method effectively verifies that state is saved and restored correctly. Consider adding more test cases to cover edge cases, such as:
- Handling of null or empty states.
- Complex state objects.
- Multiple states with different keys.
35-35: Helper class looks good.The
Holderclass is straightforward and serves its purpose well in encapsulating the state for the test.state-keeper/src/commonMain/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExt.kt (2)
16-26: Function implementation looks good.The
withSavedStatefunction provides a clear and concise way to handle saved states. The use ofconsumeandregistermethods is appropriate. Ensure that the documentation for these methods is comprehensive.
31-42: Function implementation looks good.The
withSavedStatefunction inStateKeeperOwneris a useful convenience wrapper around thewithSavedStatefunction inStateKeeper.instance-keeper/src/commonMain/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExt.kt (4)
31-33: Function implementation looks good.The
retainedInstancefunction with a key inInstanceKeeperOwneris clear and concise, delegating to thegetOrCreatemethod inInstanceKeeper.
37-38: Function implementation looks good.The
retainedInstancefunction without a key inInstanceKeeperOwneris clear and concise, delegating to thegetOrCreatemethod inInstanceKeeper.
62-63: Function implementation looks good.The
retainedSimpleInstancefunction with a key inInstanceKeeperOwneris clear and concise, delegating to thegetOrCreateSimplemethod inInstanceKeeper.
68-69: Function implementation looks good.The
retainedSimpleInstancefunction without a key inInstanceKeeperOwneris clear and concise, delegating to thegetOrCreateSimplemethod inInstanceKeeper.state-keeper/api/jvm/state-keeper.api (1)
39-42: API declarations look good.The new functions
withSavedStateinStateKeeperExtKtare consistent with the implementation in theStateKeeperExt.ktfile.state-keeper/api/android/state-keeper.api (1)
53-56: LGTM! But verify the function usage in the codebase.The added functions
withSavedStateinStateKeeperExtKtenhance the functionality by providing inline methods to handle saved states. These changes are approved.However, ensure that all function calls to
withSavedStateare correctly implemented in the codebase.Verification successful
Let's inspect the relevant lines in
StateKeeperExt.ktandStateKeeperExtTest.ktto ensure the function calls towithSavedStatematch the new signatures.
Function usage verified successfully.
The function calls to
withSavedStateinStateKeeperExt.ktandStateKeeperExtTest.ktmatch the new signatures defined in theStateKeeperExtKtclass.
state-keeper/src/commonMain/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExt.ktstate-keeper/src/commonTest/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExtTest.ktScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `withSavedState` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new functions. rg --type kotlin $'withSavedState'Length of output: 991
Script:
#!/bin/bash # Extract the lines around the usage of `withSavedState` in the relevant files to verify the function signatures. # Extract lines around usage in StateKeeperExt.kt rg --type kotlin -A 5 -B 5 'withSavedState' state-keeper/src/commonMain/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExt.kt # Extract lines around usage in StateKeeperExtTest.kt rg --type kotlin -A 5 -B 5 'withSavedState' state-keeper/src/commonTest/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExtTest.ktLength of output: 1904
state-keeper/api/state-keeper.klib.api (1)
35-36: LGTM! But verify the function usage in the codebase.The added functions
withSavedStateforStateKeeperandStateKeeperOwnerenhance the functionality by providing inline methods to handle saved states. These changes are approved.However, ensure that all function calls to
withSavedStateare correctly implemented in the codebase.Verification successful
All usages of the
withSavedStatefunction are correctly implemented in the codebase.The function calls in both
StateKeeperExtTest.ktandStateKeeperExt.ktmatch the new signatures.
state-keeper/src/commonTest/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExtTest.ktstate-keeper/src/commonMain/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExt.ktScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `withSavedState` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new functions. rg --type kotlin $'withSavedState'Length of output: 991
Script:
#!/bin/bash # Extracting relevant lines from StateKeeperExtTest.kt and StateKeeperExt.kt to verify the function usage # Extract lines around the usages in StateKeeperExtTest.kt rg --context 5 'withSavedState' state-keeper/src/commonTest/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExtTest.kt # Extract lines around the usages in StateKeeperExt.kt rg --context 5 'withSavedState' state-keeper/src/commonMain/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExt.ktLength of output: 1880
instance-keeper/api/instance-keeper.klib.api (1)
33-37: LGTM! But verify the function usage in the codebase.The added functions
retainedInstanceandretainedSimpleInstanceforInstanceKeeperOwnerenhance the functionality by providing inline methods to retain instances. These changes are approved.However, ensure that all function calls to
retainedInstanceandretainedSimpleInstanceare correctly implemented in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- instance-keeper/api/instance-keeper.klib.api (1 hunks)
- instance-keeper/src/commonMain/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExt.kt (3 hunks)
- state-keeper/api/android/state-keeper.api (1 hunks)
- state-keeper/api/jvm/state-keeper.api (1 hunks)
- state-keeper/api/state-keeper.klib.api (1 hunks)
- state-keeper/src/commonMain/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExt.kt (1 hunks)
- state-keeper/src/commonTest/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExtTest.kt (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- instance-keeper/api/instance-keeper.klib.api
- instance-keeper/src/commonMain/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExt.kt
- state-keeper/api/android/state-keeper.api
- state-keeper/api/jvm/state-keeper.api
- state-keeper/src/commonMain/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExt.kt
- state-keeper/src/commonTest/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExtTest.kt
Additional comments not posted (2)
state-keeper/api/state-keeper.klib.api (2)
35-35: LGTM! The new functionwithSavedStateis well-implemented.The function is appropriately designed for saving and restoring state using generics and serialization.
36-36: LGTM! The new functionwithSavedStateis well-implemented.The function is appropriately designed for saving and restoring state using generics and serialization.
…dInstance extensions
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- instance-keeper/api/instance-keeper.klib.api (1 hunks)
- instance-keeper/src/commonMain/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExt.kt (3 hunks)
- state-keeper/api/android/state-keeper.api (1 hunks)
- state-keeper/api/jvm/state-keeper.api (1 hunks)
- state-keeper/api/state-keeper.klib.api (1 hunks)
- state-keeper/src/commonMain/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExt.kt (1 hunks)
- state-keeper/src/commonTest/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExtTest.kt (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- instance-keeper/api/instance-keeper.klib.api
- instance-keeper/src/commonMain/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExt.kt
- state-keeper/api/android/state-keeper.api
- state-keeper/api/jvm/state-keeper.api
- state-keeper/src/commonMain/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExt.kt
- state-keeper/src/commonTest/kotlin/com/arkivanov/essenty/statekeeper/StateKeeperExtTest.kt
Additional comments not posted (2)
state-keeper/api/state-keeper.klib.api (2)
35-35: LGTM! ThewithSavedStatefunction forStateKeeperis well-defined.The function uses Kotlin's inline and reified type parameters effectively, offering flexibility in state management.
36-36: LGTM! ThewithSavedStatefunction forStateKeeperOwneris well-defined.The function is consistent with the
StateKeeperversion, maintaining flexibility and adhering to best practices.
Summary by CodeRabbit
InstanceKeeperOwnerinterface for retaining instances, enhancing theInstanceKeepermodule.withSavedStatefunctions to theStateKeeperandStateKeeperOwnerinterfaces for handling saved states with specific parameters and return types.withSavedStatefunctionality in theStateKeepermodule.