Conversation
|
@tiholic any chance of approving? |
| private final Registrar registrar; | ||
| private final StreamsChannel streamsChannel; | ||
| private FlutterPluginBinding flutterPluginBinding; | ||
| private StreamsChannel streamsChannel; |
There was a problem hiding this comment.
reason for streamsChannel not being final?
There was a problem hiding this comment.
The new Android embedding 2.0 calls the default constructor initially (very annoying) so we can not initializer the stream channel. The onAttachedToEngine calls our private constructor which initializes StreamChannel. If the default constructor did not get called or we set a dummy value for it ion this constructor then that would work but is not very elegant.
| // final AdharaSocketIoPlugin plugin = new AdharaSocketIoPlugin(registrar, streamsChannel); | ||
| // final MethodChannel channel = new MethodChannel(registrar.messenger(), PlatformConstants.MethodChannelNames.managerMethodChannel); | ||
| // channel.setMethodCallHandler(plugin); | ||
| // } |
There was a problem hiding this comment.
It would be good to avoid unwanted commented code.
There was a problem hiding this comment.
I could not agree more. I will remove.
example/lib/main.dart
Outdated
| List<String> toPrint = ['trying to connect']; | ||
| SocketIOManager manager; | ||
| List<String?> toPrint = ['trying to connect']; | ||
| SocketIOManager? manager; |
There was a problem hiding this comment.
This can be late SocketIOManager manager, thus avoiding the need to add manager! everywhere else in this file
lib/manager.dart
Outdated
| ); | ||
|
|
||
| final _sockets = <int, SocketIO>{}; | ||
| final _sockets = <int?, SocketIO>{}; |
There was a problem hiding this comment.
the key is supposed to be strict int. Any other possibilities are to be filtered out in usages below.
There was a problem hiding this comment.
The problem is that: -
final index = await _channel.invokeMethod(
PlatformMethod.newInstance,
{
'options': options.asMap(),
'clear': _clearExisting,
},
);
Returns a Future<int?> as per the MethodChannel contract so we would need to provide a default with ??.
| } | ||
|
|
||
| Completer _connectSyncCompleter; | ||
| Completer? _connectSyncCompleter; |
There was a problem hiding this comment.
You set it to null in: -
void cleanup() {
onConnectSubscription.cancel();
onConnectErrorSubscription.cancel();
_connectSyncCompleter = null;
}
So it has to have the ?
lib/socket.dart
Outdated
|
|
||
| /// checks whether connection is alive | ||
| Future<bool> isConnected() => _channel.invokeMethod( | ||
| Future<bool?> isConnected() => _channel.invokeMethod( |
There was a problem hiding this comment.
If we can implement a method that ensures returning of null-safe values like the one i created here would avoid usage of unwanted ? and !
There was a problem hiding this comment.
I will throw an exception, it does mean we have to await though.
pubspec.yaml
Outdated
|
|
||
| dev_dependencies: | ||
| adhara_socket_io_example: | ||
| path: example |
There was a problem hiding this comment.
what is the need for example as a dev dependency?
There was a problem hiding this comment.
I forgot there reason why, but I have removed it now.
| class SocketIO { | ||
| /// Socket -or- Connection identifier | ||
| final int id; | ||
| final int? id; |
|
@ihancock I see the support for minSDK version is changed from 16 to 21. What is the reason for increasing the min value? And if it is justifyable, we can also remove unwanted android versions from workflow files |
|
Is this ever going to be completed? Seems to be some issues with the |
|
It's been a while without any input. |
4 participants
First pass untested