canonical / nm.dart

Native Dart client library to access NetworkManager on Linux.
https://pub.dev/packages/nm
Mozilla Public License 2.0
29 stars 11 forks source link

Empty property change streams #52

Open jpnurmi opened 2 years ago

jpnurmi commented 2 years ago

Would it make sense to output a warning, for example, if NetworkManagerClient.propertiesChanged or other similar properties are accessed before NetworkManagerClient.connect() has completed?

final client = NetworkManagerClient();
await client.connect();
client.propertiesChanged.listen((properties) => print('changed: $properties'));
await client.setSomething(...); // "changed: [...]"
}

vs.

final client = NetworkManagerClient();
client.propertiesChanged.listen((properties) => print('changed: $properties'));
await client.connect();
await client.setSomething(...); // <nothing>

The above example may look silly because the order makes perfect sense and the fix is just a matter of swapping two lines. However, when the NM client is injected into a widget tree and lazily initialized when needed, things may get confusing when property change notifications go quietly missing when accidentally setting up subscriptions at construction time.

When the code is changed to something like this, the problem is no longer that obvious.

class NetworkViewModel extends ChangeNotifier {
  NetworkViewModel(this.client) {
    client.propertiesChanged.listen((_) => notifyListeners());
  }
  final NetworkManagerClient client;
  bool get wirelessEnabled => client.wirelessEnabled;
  Future<void> init() => client.connect().then((_) => notifyListeners());
}

class NetworkViewState extends State<NetworkView> {
  void initState() {
    super.initState();
    context.read<NetworkViewModel>().init();
  }
}
robert-ancell commented 2 years ago

It would be better if we could make these stream work regardless of when they are connected. I guess that would involve making a StreamController for each object, then using addStream() when the DBus object appears to pass the events.