Skip to content

Commit

Permalink
doc string for AblyEventStreamHandler and raise proper error on uni…
Browse files Browse the repository at this point in the history 
…mplemented event channels
  • Loading branch information
Rohit R. Abbadi committed May 7, 2020
1 parent c93bb90 commit d59f89a
Showing 1 changed file with 12 additions and 2 deletions.
14 changes: 12 additions & 2 deletions android/src/main/java/io/ably/flutter/plugin/AblyEventStreamHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,19 @@
import io.flutter.plugin.common.EventChannel;


/**
* Dart side can listen to Event Streams by pushing data to eventSink available in onListen method.
* Event listening can be cancelled when stream subscription is cancelled on dart side
*
* ref: https://api.flutter.dev/javadoc/io/flutter/plugin/common/EventChannel.StreamHandler.html
* */
public class AblyEventStreamHandler implements EventChannel.StreamHandler {
private final AblyMethodCallHandler methodCallHandler;

/**
* Constructor requiring methodCallHandler, as it is a singleton and has all instances stored
* Event listening can be started on an instance picked from the stored instances
* */
AblyEventStreamHandler(AblyMethodCallHandler methodCallHandler){
this.methodCallHandler = methodCallHandler;
}
Expand Down Expand Up @@ -78,9 +88,9 @@ public void onListen(Object object, EventChannel.EventSink uiThreadEventSink) {
ablyLibrary.getRealtime(message.handle).connection.on(connectionStateListener);
return;
case "realtime_onChannelStateChanged":
channelStateListener = new PluginChannelStateListener(eventSink);
// channelStateListener = new PluginChannelStateListener(eventSink);
// ablyLibrary.getRealtime(message.handle).connection.on(channelStateListener);
return;
// return;
default:
eventSink.error("unhandled event", null, null);
}
Expand Down

4 comments on commit d59f89a

@QuintinWillison
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies if you've moved on since but why is the code just commented out rather than deleted?

@tiholic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QuintinWillison this will be uncommented while implementing respective features. Do you prefer to be removed and re-written rather than have comments in code?
My view: we can enforce such restrictions once the library is in good shape.

@paddybyers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of everything should be clear. So if there is code that is commented-out, then at a minimum the purpose of leaving it there should be clear - at least, that means leaving a TODO and comment explaining it. Perhaps the purpose is that the commented-out code is illustrative of what the code is supposed to do, for example.

However, if the purpose is just to avoid having to re-enter it when you want to reinstate the code, then I personally would prefer that it's not there. You can get the code from the previous revision if necessary.

If you are leaving it there just to remind you to do it at a later time, then a TODO comment or an issue, or both, are I think a better way to track it.

Sometimes you will create a function or method with an empty body because you need it to exist to satisfy an interface, but that implementation is not part of the present PR. That's fine so long as there's a comment that makes it clear (eg with a TODO: unimplemented comment or similar).

@tiholic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QuintinWillison @paddybyers this commented out code has been removed in the re-codec PR #13 as a result of other discussion.

Please sign in to comment.