Commit 9ece6daf authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Add more asserts and docs to ChangeNotifier. (#7513)

It took me a while to figure out what was going on (I was removing a
listener after disposal). These asserts helped.
parent b72fa88f
...@@ -32,29 +32,57 @@ abstract class Listenable { ...@@ -32,29 +32,57 @@ abstract class Listenable {
/// A class that can be extended or mixed in that provides a change notification /// A class that can be extended or mixed in that provides a change notification
/// API using [VoidCallback] for notifications. /// API using [VoidCallback] for notifications.
///
/// [ChangeNotifier] is optimised for small numbers (one or two) of listeners.
/// It is O(N) for adding and removing listeners and O(N²) for dispatching
/// notifications (where N is the number of listeners).
class ChangeNotifier extends Listenable { class ChangeNotifier extends Listenable {
List<VoidCallback> _listeners; List<VoidCallback> _listeners;
bool _debugAssertNotDisposed() {
assert(() {
if (_listeners == const <VoidCallback>[]) {
throw new FlutterError(
'A $runtimeType was used after being disposed.\n'
'Once you have called dispose() on a $runtimeType, it can no longer be used.'
);
}
return true;
});
return true;
}
/// Register a closure to be called when the object changes. /// Register a closure to be called when the object changes.
///
/// This method must not be called after [dispose] has been called.
@override @override
void addListener(VoidCallback listener) { void addListener(VoidCallback listener) {
assert(_debugAssertNotDisposed);
_listeners ??= <VoidCallback>[]; _listeners ??= <VoidCallback>[];
_listeners.add(listener); _listeners.add(listener);
} }
/// Remove a previously registered closure from the list of closures that are /// Remove a previously registered closure from the list of closures that are
/// notified when the object changes. /// notified when the object changes.
///
/// If the given listener is not registered, the call is ignored.
///
/// This method must not be called after [dispose] has been called.
@override @override
void removeListener(VoidCallback listener) { void removeListener(VoidCallback listener) {
assert(_debugAssertNotDisposed);
_listeners?.remove(listener); _listeners?.remove(listener);
} }
/// Discards any resources used by the object. After this is called, the object /// Discards any resources used by the object. After this is called, the
/// is not in a usable state and should be discarded. /// object is not in a usable state and should be discarded (calls to
/// [addListener] and [removeListener] will throw after the object is
/// disposed).
/// ///
/// This method should only be called by the object's owner. /// This method should only be called by the object's owner.
@mustCallSuper @mustCallSuper
void dispose() { void dispose() {
assert(_debugAssertNotDisposed);
_listeners = const <VoidCallback>[]; _listeners = const <VoidCallback>[];
} }
...@@ -67,8 +95,11 @@ class ChangeNotifier extends Listenable { ...@@ -67,8 +95,11 @@ class ChangeNotifier extends Listenable {
/// ///
/// Exceptions thrown by listeners will be caught and reported using /// Exceptions thrown by listeners will be caught and reported using
/// [FlutterError.reportError]. /// [FlutterError.reportError].
///
/// This method must not be called after [dispose] has been called.
@protected @protected
void notifyListeners() { void notifyListeners() {
assert(_debugAssertNotDisposed);
if (_listeners != null) { if (_listeners != null) {
List<VoidCallback> localListeners = new List<VoidCallback>.from(_listeners); List<VoidCallback> localListeners = new List<VoidCallback>.from(_listeners);
for (VoidCallback listener in localListeners) { for (VoidCallback listener in localListeners) {
......
...@@ -181,4 +181,13 @@ void main() { ...@@ -181,4 +181,13 @@ void main() {
source2.notify(); source2.notify();
expect(log, isEmpty); expect(log, isEmpty);
}); });
test('Cannot use a disposed ChangeNotifier', () {
final TestNotifier source = new TestNotifier();
source.dispose();
expect(() { source.addListener(null); }, throwsFlutterError);
expect(() { source.removeListener(null); }, throwsFlutterError);
expect(() { source.dispose(); }, throwsFlutterError);
expect(() { source.notify(); }, throwsFlutterError);
});
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment