-
Notifications
You must be signed in to change notification settings - Fork 2
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
EventedMapper as a proxy #74 #3
base: master
Are you sure you want to change the base?
Conversation
default: | ||
$message = 'Invalid type of action provided for event manager'; | ||
throw new \InvalidArgumentException($message, 400); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this switch?
Can we use if
instead?
if (!(is_callable($action)) || !($action instanceof Listener)) {
$message = 'Invalid type of action provided for event manager';
throw new \InvalidArgumentException($message, 400);
}
// The block below you can use for control or remove it
$events = $this->getEvents();
if (array_key_exists("attach", $events)) {
$this->dispatch("attach", array($eventName, $action));
}
$this->events[$eventName][] = $action;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably for future implementations or checks. I don't see any trouble by changing it.
@Rafael-BP could you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, no problem.
As described under this issue under Respect\Relational there's an implementation of a Proxy interface to add events onto Respect\Data.
This could be user as this:
Same goes for every events: insert, update, delete. With suffixes: pre and post.
So there are six events to be listened: