Subscription count event #331
Conversation
singhashmeet force-pushed the SubscriptionCountEvent branch from b6f5fdf to 6842c09 Compare 
Codecov Report
@@ Coverage Diff @@ ## master #331 +/- ## ============================================ - Coverage 75.14% 74.19% -0.95% Complexity 346 346 ============================================ Files 50 51 +1 Lines 1955 1980 +25 Branches 149 151 +2 ============================================ Hits 1469 1469 - Misses 422 446 +24 - Partials 64 65 +1
Continue to review full report at Codecov.
|
| * @param count | ||
| * The latest count of subscribers on the channel. | ||
| */ | ||
| void onSubscriptionCountChanged(String channelName, int count); |
There was a problem hiding this comment.
I can't bring myself to let it go like this. This makes it harder for anyone who wants to upgrade, even if they don't want to use the feature.
I know the way it's currently designed is annoying to work with. But let's create a new interface that extends this.
e.g. ChannelWithSubscriptionCountEventListener
You probably have to add two more interfaces that extend that for private channels too. PrivateChannelWithSubscriptionCountEventListener, PrivateEncryptedChannelWithSubscriptionCountEventListener
It's pretty ugly but at least it doesn't force everyone to change their implementation.
Sorry for the late feedback.
We will need to rethink and redesign how these event listeners are added in the next major release.
There was a problem hiding this comment.
The SDK is not designed for this to happen elegantly and will complicate the integration process a lot for subscription count.
If not breaking the older implementations is a non-negotiable then I would suggest removing this event listener entirely and renaming the event to pusher:subscription_count so it can be handled with the .bind() function like in JS SDK
What do you think ?
Description of the pull request
...
Why is the change necessary?
...
CC @pusher/mobile