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.

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

Made some processes atomic. #8

Merged
merged 2 commits into from
Jun 9, 2018

Conversation

bob1de
Copy link
Contributor

@bob1de bob1de commented Jun 3, 2018

  • Prevent events triggered by event handlers from triggering handlers
    registered via once() again.
  • Prevent handlers registered from inside event handlers from firing for
    the original event as well.

- Prevent events triggered by event handlers from triggering handlers
  registered via once() again.
- Prevent handlers registered from inside event handlers from firing for
  the original event as well.
@bob1de
Copy link
Contributor Author

bob1de commented Jun 3, 2018

Another implication of this bug is the following:

>>> l = [1,2,3]
>>> for i in l:
...   print(i)
...   del l[0]
... 
1
3

If you imagine this was the list of event handlers and handler 1 (maybe conditionally) unregisters itself, handler 2 would not be executed.

The same thing happens when you first register a handler with once() and then a second one for the same event. The second handler doesn't get executed in the first run.

>>> import observable
>>> obs = observable.Observable()
>>> obs.once("some", lambda: print("first handler"))
<function Observable.once.<locals>._once_wrapper.<locals>._wrapper at 0x7f35e1206268>
>>> obs.once("some", lambda: print("second handler"))
<function Observable.once.<locals>._once_wrapper.<locals>._wrapper at 0x7f35e0b14378>
>>> obs.trigger("some")
first handler
True
>>> obs.trigger("some")
second handler
True

@bob1de
Copy link
Contributor Author

bob1de commented Jun 3, 2018

With the fix in place, the behaviour is as expected, because a copy of the handler list is created before executing them one by one.

>>> import observable
>>> obs = observable.Observable()
>>> obs.once("some", lambda: print("first handler"))
<function Observable.once.<locals>._once_wrapper.<locals>._wrapper at 0x7f54cb1cc268>
>>> obs.once("some", lambda: print("second handler"))
<function Observable.once.<locals>._once_wrapper.<locals>._wrapper at 0x7f54caadb378>
>>> obs.trigger("some")
first handler
second handler
True
>>> obs.trigger("some")
False

@bob1de
Copy link
Contributor Author

bob1de commented Jun 8, 2018

Closes #9

@bob1de
Copy link
Contributor Author

bob1de commented Jun 8, 2018

Closes #10

@timofurrer timofurrer merged commit eb393e9 into timofurrer:master Jun 9, 2018
@timofurrer
Copy link
Owner

Looks good! Thanks 🎉

@bob1de bob1de deleted the atomic_triggering branch June 9, 2018 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants