Skip to content
This repository has been archived by the owner on Aug 25, 2022. It is now read-only.

Simple event handler implementation (with @Gounlaf) #39

Merged
merged 4 commits into from
Apr 8, 2014
Merged

Simple event handler implementation (with @Gounlaf) #39

merged 4 commits into from
Apr 8, 2014

Conversation

iamslan
Copy link
Contributor

@iamslan iamslan commented Mar 25, 2014

Hi there,

Here is a simple implementation of an event handler.

Made with ❤

Contributor: @Gounlaf

} else {
foreach ($this->callbacks[$event] as $existingCallback) {
if ($existingCallback === $callback) {
$this->stdout('debug', 'Skip existing callback');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure that a comparision like that is really accurate...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comparison works for simple function (and should works with object); closure are logically not equals

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in_array can be more accurate ;

in both case (current code, or in_array), we must handle the case where $callback is a string (eg, the name of a function)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I also think that in_array may be a better choice, as it is natively implemented

@guillaumepotier
Copy link
Member

That seems great now. Thanks.

guillaumepotier added a commit that referenced this pull request Apr 8, 2014
Simple event handler implementation (with @Gounlaf)
@guillaumepotier guillaumepotier merged commit 5ae11be into Wisembly:master Apr 8, 2014
@mmt78 mmt78 mentioned this pull request Dec 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants