Deprecated InstanceKeeper#getOrCreate, added InstanceKeeper#provide and InstanceKeeper#providing extension functions #161
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe Changes
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (4)
- instance-keeper/api/android/instance-keeper.api (1 hunks)
- instance-keeper/api/jvm/instance-keeper.api (1 hunks)
- instance-keeper/src/commonMain/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExt.kt (2 hunks)
- instance-keeper/src/commonTest/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExtTest.kt (1 hunks)
Additional comments: 16
instance-keeper/src/commonTest/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExtTest.kt (4)
- 13-15: The test
WHEN_provide_with_key_called_second_time_THEN_returns_same_instancecorrectly verifies that theprovidefunction returns the same instance when called with the same key. This aligns with the expected behavior of theprovidefunction, ensuring instance uniqueness per key.- 21-23: The test
WHEN_provide_without_key_called_second_time_THEN_returns_same_instanceeffectively checks that theprovidefunction without a specified key behaves as expected, returning the same instance upon subsequent calls. This test is crucial for validating the default key behavior of theprovidefunction.- 29-31: The test
WHEN_provideSimple_with_key_called_second_time_THEN_returns_same_instanceaccurately ensures thatprovideSimplewith a key returns the same instance on repeated calls. This test is essential for verifying the correct behavior of theprovideSimplefunction, which is intended for simpler use cases without the need for instance destruction.- 37-39: The test
WHEN_provideSimple_without_key_called_second_time_THEN_returns_same_instanceconfirms thatprovideSimplewithout an explicit key returns the same instance on subsequent calls. This test validates the behavior ofprovideSimplein scenarios where a key is not provided, ensuring consistency with the expected functionality.instance-keeper/api/jvm/instance-keeper.api (1)
- 32-35: The addition of
provide,provideSimple,providing, andprovidingSimplefunctions to theInstanceKeeperExtKtclass is correctly implemented. These functions extend the functionality of theInstanceKeepercomponent, offering more flexible and intuitive options for instance management. It's important to ensure that these functions are well-documented and their usage is clear to developers.instance-keeper/api/android/instance-keeper.api (1)
- 39-42: The addition of
provide,provideSimple,providing, andprovidingSimplefunctions to theInstanceKeeperExtKtfor the Android target mirrors the changes made for the JVM target. This consistency across platforms is crucial for a unified developer experience. Ensure that these functions are accompanied by appropriate documentation and examples to facilitate their adoption.instance-keeper/src/commonMain/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExt.kt (10)
- 9-18: The deprecation of
getOrCreatein favor ofprovideis correctly implemented with a clear deprecation message and aReplaceWithaction. This approach facilitates a smooth transition for developers to the new API.- 24-24: The
providefunction is well-implemented, ensuring type safety and correct instance management. It's important to maintain comprehensive documentation for this function, explaining its behavior and usage scenarios.- 39-40: The lazy variant
providingis correctly implemented, offering a deferred instance creation option. This addition enhances the flexibility of theInstanceKeepercomponent. Ensure that the lazy behavior is clearly documented for developers.- 46-55: The deprecation of
getOrCreatefor the reified type variant in favor ofprovideis correctly handled, with a clear message andReplaceWithdirective. This consistency in deprecation strategy across different function variants is commendable.- 61-62: The reified type variant of
provideis correctly implemented, leveraging Kotlin's reified type parameters for a more intuitive API. Documentation should clearly explain the use of reified types and the benefits they offer in this context.- 77-86: The deprecation of
getOrCreateSimplein favor ofprovideSimpleis handled well, with a clear deprecation message and aReplaceWithaction. This change aligns with the overall goal of improving the API's intuitiveness and consistency.- 94-96: The
provideSimplefunction is implemented correctly, catering to simpler use cases where instance destruction is not required. It's crucial to document this function's intended use cases and limitations clearly.- 104-105: The lazy variant
providingSimpleis a valuable addition, offering deferred creation for simple instances. Documentation should highlight the lazy behavior and its potential use cases to guide developers effectively.- 113-122: The deprecation of
getOrCreateSimplefor reified types in favor ofprovideSimpleis consistent with the deprecation strategy applied to other variants. This clear and consistent approach aids developers in transitioning to the new API.- 130-131: The reified type variant of
provideSimpleis well-implemented, providing a straightforward API for simple instance management. Ensure that the benefits of using reified types in this context are well-documented.
…nd InstanceKeeper#providing extension functions.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- instance-keeper/api/android/instance-keeper.api (1 hunks)
- instance-keeper/api/jvm/instance-keeper.api (1 hunks)
- instance-keeper/src/androidUnitTest/kotlin/com/arkivanov/essenty/instancekeeper/AndroidInstanceKeeperTest.kt (3 hunks)
- instance-keeper/src/commonMain/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExt.kt (2 hunks)
- instance-keeper/src/commonTest/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExtTest.kt (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- instance-keeper/api/android/instance-keeper.api
- instance-keeper/api/jvm/instance-keeper.api
- instance-keeper/src/commonMain/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExt.kt
- instance-keeper/src/commonTest/kotlin/com/arkivanov/essenty/instancekeeper/InstanceKeeperExtTest.kt
Additional comments: 3
instance-keeper/src/androidUnitTest/kotlin/com/arkivanov/essenty/instancekeeper/AndroidInstanceKeeperTest.kt (3)
- 17-20: The replacement of
getOrCreatewithprovidein theretains_instancestest method aligns well with the PR objectives, ensuring that instances are managed more intuitively. The test logic correctly verifies that instances are retained across differentinstanceKeepercalls.- 29-32: The test
GIVEN_discardRetainedInstances_is_true_on_restore_THEN_instances_not_retainedcorrectly verifies that instances are not retained whendiscardRetainedInstancesis set to true. The use ofprovidefor instance creation is consistent with the PR's objectives and enhances the clarity of instance management.- 41-41: The test
GIVEN_discardRetainedInstances_is_true_on_restore_THEN_old_instances_destroyedeffectively ensures that old instances are destroyed whendiscardRetainedInstancesis true, aligning with the PR's focus on efficient instance management. The adoption ofprovidefor instance creation is appropriate and supports the PR's goals.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
InstanceKeepermodule, enhancing flexibility and usability.getOrCreatefunctions in favor of new, more intuitively namedprovidefunctions, including lazy variants for deferred instance creation.