Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix behavior in removeObserver. #349

Merged
merged 3 commits into from
Mar 11, 2021
Merged

Fix behavior in removeObserver. #349

merged 3 commits into from
Mar 11, 2021

Commits on Mar 11, 2021

  1. Fix behavior in removeObserver. 

    The change I made to allow `None` to indicate "please remove everything that observer is watching in observable" had a logic flaw.

- In `addObserver`, `notification=None` means "send every notification to this observer for this observable".
- In `removeObserver`, `notification` means "remove every registered notification from this observable for this observer".

Those seem alike, but this example shows my mistake:

```python
er = This()
able = That()

able.addObserver(observer=er, notification=None, method="anyNotificationHappened")
able.addObserver(observer=er, notification="specific", method="specificNotificationHappened")

able.removeObserver(observer=er, notification=None)
```

The `notification=None` has two conflicting meanings:

1. Remove the notification that will call method `er.anyNotificationHappened`.
2. Remove all observations for er.
    - `er.anyNotificationHappened`
    - `er.specificNotificationHappened`

Being able to vigorously remove observations without knowing specific notifications is useful, but the previous behavior is more important. So, I'm introducing an "all" option for `notification` to provide the new functionality without changing the old.
    @typesupply
    typesupply committed Mar 11, 2021
    Configuration menu
    Copy the full SHA
    ab9c9f1 View commit details
    Browse the repository at this point in the history

  2. Be more suspicious when removing observers from the backtrack dict. S… 

    …ometimes things (cough tests cough) remove things that haven't been registered.
    @typesupply
    typesupply committed Mar 11, 2021
    Configuration menu
    Copy the full SHA
    495d179 View commit details
    Browse the repository at this point in the history

  3. Don't let 'all' come in accidentally.

    @typesupply
    typesupply committed Mar 11, 2021
    Configuration menu
    Copy the full SHA
    5dccf65 View commit details
    Browse the repository at this point in the history