New event system or not

In this post, phpnode suggested a new event system which is supposedly to introduce more flexibility to Yii. Because this would be a major change, I would like to hear your opinion before we commit to implement it.

New Event System

  • No declaration is needed for events. As long as there’s a call of $component->raiseEvent($eventName), we say an event named $eventName exists for $component.
  • We no longer support $component->$eventName = $callback; syntax for binding event handlers. One has to use attachEventHandler() or $component->eventHandlers to bind handlers.
  • Pros: 1. this offers component behaviors opportunity to define new events for the component. 2. Eliminate the need of redundant event declaration (via on-method).
  • Cons: more error prone when binding and raising events because event names are no longer validated.

I would love to see some improvements on the event system. Adding events dynamically using behaviors is an interesting aspect. I can also remember I had some problems understanding yii’s event system when I looked at it the first time, so more room for improvements. I think part of that confusion was that I had to assign a callback to an existing method…

Don’t get me wrong: I prefer explicitly declared events over the raise(something) approach. I want the events I use to be documented, supported by my IDE and I want them to be validated. It’s only that assign-to-method, which I think caused confusion. But this is only event registration, only part of the whole, don’t want to focus on that right now…

What I would appreciate were some additional features like static events:




User::onRegistrationConfirmed = array( $this, 'sendWelcomeMessage' );



and inter-component events:




return $appConfig = array(

  'connections' => array(

    array( 'module.user', 'onUserRegistered', 'module.admin', 'queueForConfirmation' ),

  ),

);



Besides this, I think there should be more discussion about what events should be used for than about how they should be defined. I personally like to think of events as notifications. “Hey there! Something happened!”. That’s why I love Qt’s signals. Their single purpose is to notify. Without any knowledge (or interest) about who or what or if something is listening to them. They don’t have a return value. Nor do they check a “handled” flag (at least I don’t know of something like this).

Yii’s events are different. They are often used to provide extension points. “Hey there! Currently I’m in this state! Prepared me? Ok, I’m moving on! Hey there, me again! Now I’m in this state…”.

You see, these are two different things. The one is about notifications, the other about customizing behavior. Maybe only two use cases for one mechanism, but I wonder if maybe it shouldn’t be two separate ones.

Oh, about the inter-component events:

I posted a proof of concept implementation in an older thread about events.

and code will tend to become messy and less documented. I think it’s better when a component defines all its events and behavior can only listen to/raise events defined in the component, so you can see all events of the component by looking on its code.

if behavior need some event to be defined in the component then this fact can be one of behaviors’ requirements - this will be more clear. And if the behavior is widely used in your appliacation and it requires some event defined in several components then you can define them using traits (PHP 5.4 is ready) to prevent code duplication, and this trait can be also distributed with the behavior when making an extension

It’s my vision of the question and I still prefer the current event system over the new suggested

The problem is, this is very restrictive, often the components you want to attach new events to are framework components, and it’s generally a bad idea to edit framework code. You can’t add traits to the framework components without editing the framework code, traits cannot be attached at run time, so I think a more flexible event system is useful. There is no problem documenting events with a phpdoc @event tag.

yes I agree that editing of framework code is generally a bad idea, but you can extend (not edit) framework classes and declare needed events in children classes. maybe sometimes it will not be possible to extend some class and make other framework components using your new class instead of the original, but this situation is a design fault I think and should be fixed by core team by making possible to use dependency injection

I don’t see a problem with this, is there a problem at all?

yes, but it’s not so clear as onSomething() method and if @event tag is omitted then it will be very easy to miss some existing event

I think that this flexibility doesn’t solve some problem but rather allows to make more hard-to-understand design and code, it’s like multiple inheritance (don’t want holy wars :) just describing my opinion)

Just my two cents about the raiseEvent / attachEvent stuff.

I created some time ago something similar in js (you can use it in sweekit extension or see it here : source https://github.com/pgaultier/sweekit/blob/master/js/source/jquery.sweelix.callback.js , doc : https://github.com/pgaultier/sweekit/blob/master/doc/javascript.mkd)

In my case, events are validated at runtime which means :

  • if event handler exists he is called and nothing appears

  • if event handler does not exists, a warning (in the logging system) explaining it is raised

This way I’m able to raise (js) events everywhere even if they don’t have an handler. This way my js is “loosely coupled”.

If I understand you proposal correctly, Yii2.0 would implement something similar.

This is a great feature because which allows the development to be more modular but we will need to be able to trace (in the logs or anything else) all the raiseEvent statements to be sure all event raised have a target handler or are in use.

This is important to avoid keeping dead code.

@kiwisoft - the problem with that is that you’d get loads of items in the log whenever the framework raises an event. You shouldn’t be forced to handle everything

@phpnode - Well, you are right about the “load of items in the log”. In fact I wasn’t thinking about logs but about some kind of db/store where you can store during a test run all the event raised and if they where handled (at least once) or not.

Hi!

I think we need to define use cases to find out what can be done with the current event system and what not(and if we want to do/allow this at all). We should not create something different and new just because we are creating 2.0. We should keep what was good.

I like the current implementation because it can be well documented, auto completed by IDE and validated. It uses native PHP constructs so it is very clear which events are there and how they work. This new system would implement another layer of magic that is not clearly visible in code.

And also I really loved syntax like


$component->$eventName = $callback;

in my configuration.

To come to the list of things that are not possible with current implementation:

The only valid use case I could find in the discussion till now is the point that behaviors could want to add events to a class. This can easily be done with the current implementation by adding some code to CComponent methods that goes through behaviors and checks if event is declared there.

I also like the idea of global events but this might need another solution since there are many cases someone would want to use it for. Also these events should be well declared when they are used by more than one component. One could implement an application Component GlobalEventManager and implement the events there. Also registerEvent() to have dynamic events would be possible there. Not sure if this one should be part of the framework since it has to be very specifically designed for the use case.

Can’t think of any use case where I would want to create Events at runtime… please provide examples!

best regards,

cebe

Let’s say you have a State machine that you can attach to Article models. Articles can be draft, published or archived. You want to encapsulate all your business logic into different state classes. When an article is read by a user, you want to raise an onRead() event on it, but articles can only be read once they’ve been published, so you want to put that logic in your PublishedArticleState class. The problem is your event will have to be invoked in the context of PublishedArticleState, not the Article model it belongs to, because at the moment, new events cannot be added on the fly. This means I can’t do




$article->on("read", function($event) { foo($event->sender); }); 



I have to do




if ($article->status->is("published")) {

    $article->getState()->on("read", function($event) { foo($event->sender->owner); });

}



It’s just more verbose, I like the more flexible approach, it encourages people to actually use events because they don’t have to write 2 separate methods for every event they implement.

Before I knew about Yii I’ve implemented events in a project using naming convention. I can’t remember all the details, but it looks something like this:




class CComponent {

	function trigger($eventName, $params = null) {

		$this->raise('before' . $eventName, $params);	// calls all registered callbacks

		$method = 'event' . $eventName;

		$this->$method($params);

		$this->raise('after' . $eventName, $params);

	}


	function registerEventHandler(...) {...};

	function unregisterEventHandler(...) {...};

}


class CMyClass extends CComponent {

	function eventSave($params) { ... };	// in Yii this would be: function save(...) {...};

}


// save an instance of CMyClass

$params = ...;	// often $params can be omitted

myClass->trigger('save', $params);



This way it is still possible to attach handlers using a construct like

myClass->onBeforeSave = myHandler;

It is also possible to trigger the event using a constuct like:

myClass->save($params);

using the magic __call method.

Having all of this said I need to mention (to clarify my thoughts) I implemented a separate hook system which was quite similar (yet slightly different) to the current Yii event system. The characteristics of these two systems:

Events: asynchronous (or pretend to be in PHP), cannot interfere with program logic.

Hooks: synchronous, can interfere with program logic (like changing an SQL statement or changing a name).

If it decreases performance, then I would answer no.

I can’t vote in the poll because I am looking for use cases.

Same as jacmoe. If it will decrease performance significantly than I’d prefer not to have it in the core.

This is extremely annoying in my opinion. +1 for the new event system.

It’s actually significantly faster based on some tests i just did (please feel free to pick them apart). Source code is at: https://github.com/phpnode/yii/compare/test-new-event-syntax

Results:

Traditional: 10000 iterations in 0.3963840007782 seconds.

New Syntax: 10000 iterations in 0.099419832229614 seconds.

2 Callbacks Traditional: 10000 iterations in 0.44100999832153 seconds.

2 Callbacks New Syntax: 10000 iterations in 0.1295440196991 seconds.

one possible use case is not about runtime ,it’s about future program ,we will introduce some unknown event in the future ,if your component is in your “core” subSystem we normally don’t want to change the core ,if use the current event system we must declare the new coming event to the component in the “core”,so i think the event without declare is useful sometimes。

we can retain the current event system and add another coexist event system(just introduce the Mediator Pattern , now yii implement the Observer Pattern)as samdark referred plugin design

A difference in your benchmark is that you are using an array to keep event handlers while in CComponent, it is a CList (which allows you to set the event handler priority). I think this needs to be addressed to get a fair comparison.

Updated to use CList, it’s still faster but not quite as much:

Traditional: 10000 iterations in 0.35174894332886 seconds.

New Syntax: 10000 iterations in 0.2248899936676 seconds.

2 Callbacks Traditional: 10000 iterations in 0.44050002098083 seconds.

2 Callbacks New Syntax: 10000 iterations in 0.29626202583313 seconds.

What about memory usage?

Could you include that in your benchmark, please? :)

Ok, i’ve updated the code with some tests for memory usage:

Old Syntax: 10000 iterations in 0.34604716300964 seconds (8406184 bytes)

New Syntax: 10000 iterations in 0.22934198379517 seconds (8406976 bytes)

There is basically no difference in memory usage between the two. I also made things fairer by raising CModelEvent instead of CEvent in both methods, the performance gap increases slightly:

Traditional: 10000 iterations in 0.46796894073486 seconds

New Syntax: 10000 iterations in 0.30651998519897 seconds.

2 Callbacks Traditional: 10000 iterations in 0.5995180606842 seconds.

2 Callbacks New Syntax: 10000 iterations in 0.35073399543762 seconds.