Removed unused type parameters from the experimental PredictiveBackParams #777
Conversation
WalkthroughThe changes primarily focus on the Changes
Sequence Diagram(s)sequenceDiagram
participant A as User
participant B as StackAnimation
participant C as PredictiveBackParams
A->>B: Request stack animation
B->>C: Get PredictiveBackParams
C-->>B: Return PredictiveBackParams
B-->>A: Provide stack animation
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (5)
Files skipped from review due to trivial changes (1)
Files skipped from review as they are similar to previous changes (4)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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: 1
Outside diff range and nitpick comments (1)
extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/StackAnimation.kt (1)
37-37: Consider the trade-offs of simplifying thepredictiveBackParamsparameter type.The removal of generic type parameters
CandTfrom thePredictiveBackParamsreturn type inpredictiveBackParamssimplifies the API for callers. However, this change also has some drawbacks:Pros:
- Callers no longer need to specify the generic types for
PredictiveBackParams, which makes the API easier to use.Cons:
- The loss of type safety could lead to runtime errors if the returned
PredictiveBackParamsinstance does not match the expected typesCandT.- Changing from a nullable to a non-nullable return type is a breaking change for callers who were previously returning
null. They will now need to return a non-nullPredictiveBackParamsinstance, which may require significant refactoring.Please consider these trade-offs and the potential impact on existing callers before proceeding with this change. If the simplification benefits outweigh the drawbacks, then this change can be approved.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- extensions-compose-experimental/api/extensions-compose-experimental.klib.api (2 hunks)
- extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt (3 hunks)
- extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackParams.kt (1 hunks)
- extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/StackAnimation.kt (2 hunks)
Additional comments not posted (7)
extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackParams.kt (1)
23-23: Simplification of thePredictiveBackParamsclass.The removal of the generic type parameters
<in C : Any, in T : Any>simplifies thePredictiveBackParamsclass definition and usage. This change allows for a broader usage of the class, as it no longer enforces specific type constraints on its properties.However, it's important to consider the potential impact of this change:
- Instances of
PredictiveBackParamsmay now be created and used differently in the codebase.- The compiler will no longer enforce the previously defined type constraints on the class properties, which may have type safety implications.
Overall, the change is unlikely to cause any runtime issues, as the class properties themselves remain unchanged.
extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/StackAnimation.kt (1)
59-59: Consider the trade-offs of simplifying thepredictiveBackParamsparameter type.The changes made to this
stackAnimationfunction are similar to the previous one. Please refer to the comment on line 37 for a detailed analysis of the trade-offs and potential impact on callers.extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt (2)
178-178: LGTM!The change in the
predictiveBackParamsparameter type aligns with the modifications made to thePredictiveBackParamsclass, promoting consistency and simplifying usage.
220-220: Looks good!The change in the
predictiveBackParamsproperty type is consistent with the modifications made to thePredictiveBackParamsclass and other usages within the file, ensuring a cohesive and simplified API.extensions-compose-experimental/api/extensions-compose-experimental.klib.api (3)
21-21: LGTM!The removal of generic type parameters from the
PredictiveBackParamsclass declaration simplifies the API and potentially leads to a more straightforward usage pattern.
39-39: LGTM!The removal of generic type parameters from the return type of the
PredictiveBackParamsargument in thestackAnimationfunction signature enhances type safety and reduces complexity in function calls. This change reflects an effort to streamline the API and may lead to easier integration and implementation in client code.
40-40: This change is similar to the one made in the previous code segment at line 39 and has been reviewed there.
...arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt
Show resolved
Hide resolved
arkivanov
force-pushed
the
remove-generics-from-PredictiveBackParams
branch
from
5195b4c to
c91435f
Compare
Summary by CodeRabbit
New Features
PredictiveBackParamsby removing generic type parameters, enhancing usability.PredictiveBackParamsis non-nullable.Bug Fixes