Closed ttypic closed 1 month ago
The changes introduced in this pull request primarily involve modifications to several interfaces within the chat application, focusing on listener management and cancellation mechanisms. The existing methods for unregistering listeners have been removed, and corresponding methods have been updated to return a Subscription
object, streamlining the subscription process. Additionally, a new Subscription
interface has been added to formalize the unsubscription process.
Files | Change Summary |
---|---|
.gitignore |
Added /.idea/shelf to ignore JetBrains IDEs' shelved changes. |
chat-android/src/main/java/com/ably/chat/*.kt |
Modified interfaces (ConnectionStatus , EmitsDiscontinuities , Messages , Occupancy , Presence , RoomReactions , RoomStatus , Typing ) to return Subscription objects for subscription methods, removing the need for separate unsubscribe methods. Added a new Subscription interface for managing listener unsubscriptions. |
Messages.kt
and Occupancy.kt
are related to modifications in listener management and the introduction of a Subscription
object for subscription methods.In the meadow where bunnies play,
Changes hop in, brightening the day.
With cancellations clear and neat,
Listener management can't be beat!
So let's celebrate with a joyful cheer,
For cleaner code is finally here! 🐇✨
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?
@ttypic I am skeptical about use of Cancellation object
. I believe this comes from multithreading/synchronization concept and used for managing thread/coroutine synchronization. We have similar concept in
While both of those languages are pretty old and natively support passing cancellation tokens, those cancel tokens are only used to manage dotnet tasks / goroutines. In both ably-dotnet and ably-go, we return unsubscribe
function to de-register listeners.
In my honest opinion, this will cause naming asymmetry across SDKs and eventually lead to more confusion.
PS. It is confusing for me too : (
@sacOO7 Not sure what are you suggesting? And I don't see why we have to have 2 different functional interfaces for unsubscription (one with unsubscribe()
another with off()
). It's not a java or kotlin way. Single interface for subscription cancellation will help us generate extensions for coroutines easier later.
Also I expect that chat sdk users won't use this methods at all, we'll provide extensions for coroutines and maybe some other reactive frameworks.
@sacOO7 Not sure what are you suggesting? And I don't see why we have to have 2 different functional interfaces for unsubscription (one with
unsubscribe()
another withoff()
). It's not a java or kotlin way. Single interface for subscription cancellation will help us generate extensions for coroutines easier later.Also I expect that chat sdk users won't use this methods at all, we'll provide extensions for coroutines and maybe some other reactive frameworks.
Okay, can we use some other keyword instead of cancel
? If we are calling subscribe to add listener
at top level, it will return unsubscribe
to cancel the subscription. Use of cancel
keyword is confusing in a way that, it used to manage/cancel coroutines on one hand and here we are using it for ably specific operations.
In this PR, I refactored the subscription pattern to align with the JavaScript SDK. Initially, I thought having two separate methods for subscribing and unsubscribing would be more idiomatic for Kotlin/Java. However, it seems that modern libraries have started adopting the cancellation object approach.
Now, instead of using two methods, a cancellation object is returned upon subscription, which can be used to unsubscribe. This simplifies the API and introduces a unified cancellation interface, making it consistent across the codebase. It also lays the groundwork for adding convenient features later, like the
use
method in Kotlin for cleaner syntax and resource management.Summary by CodeRabbit
New Features
Subscription
interface for managing subscriptions and event listeners, enhancing resource management.Subscription
object, allowing for easier cancellation of subscriptions.Bug Fixes
unsubscribe
methods across multiple interfaces to streamline listener management.Refactor
ConnectionStatus
,EmitsDiscontinuities
, and others.