Skip to content

Conversation

msabramo
Copy link

@msabramo msabramo commented Apr 4, 2012

The "originator" is the object and/or file and line number of the code that
called EventDispatcher::dispatch

This could be useful for debugging although my original motivation for
this is that in my project I want the listener to be able to find out
about objects that it otherwise wouldn't know about, because the objects
are created after the listener is attached.

In my specific example, I am creating for our proprietary MVC framework a debug toolbar much like Symfony's and I need a listener that is called when a view is rendered and I need that listener to be able to find out the view object that dispatched the event. This allows me to do that without subclassing Event.

"originator" is the object and/or file and line number of the code that
called EventDispatcher::dispatch

This could be useful for debugging although my original motivation for
this is that in my project I want the listener to be able to find out
about objects that it otherwise wouldn't know about, because the objects
are created after the listener is attached.
@stof
Copy link
Member

stof commented Apr 4, 2012

pull requests should be sent to symfony/symfony. The subtree splits are marked as read-only in their description, which means that only the cron job updating them from the symfony repo writes in them.

there is currently a discussion to see if it could be possible to accept pull requests coming both ways but we haven't set up the needed tools yet (or even figured if it can be done reliably)

@stof
Copy link
Member

stof commented Apr 4, 2012

Btw I vote against it. This makes the behavior of the event dispatcher dependant of the way it is called, meaning it is not testable and is not a clean OO design (the behavior of a method should not depend of the piece of code calling it).

If you want to pass data about the place triggering the event, you should probably do it explicitly.

@msabramo
Copy link
Author

msabramo commented Apr 4, 2012

Closing because I submitted to the wrong repo and no need I think to file another since this already got two downvotes (one from Drak on the symfony-devs mailing list).

@msabramo msabramo closed this Apr 4, 2012
@msabramo
Copy link
Author

msabramo commented Apr 4, 2012

@stof Thanks for the comment. Good points. The one thing about passing the data explicitly is that it feels a little heavy and not so DRY. As I understand it, I would have to subclass Event to create an event that stores extra data. And then I would either have to explicitly call "new EventWithData($this)" in my app code when dispatching (coupling my code to this new event class) or subclass EventDispatcher to create EventWithData instances instead of Event instances. I don't like either of these approaches very much. Maybe there is another way I am overlooking?

I feel like the Event class could really use at least a simple way to attach user data.

@msabramo
Copy link
Author

msabramo commented Apr 4, 2012

On Wed, Apr 4, 2012 at 8:49 AM, Christophe Coevoet <
reply@reply.github.com

wrote:

Btw I vote against it. This makes the behavior of the event dispatcher
dependant of the way it is called, meaning it is not testable and is not a
clean OO design (the behavior of a method should not depend of the piece of
code calling it).

If you want to pass data about the place triggering the event, you should
probably do it explicitly.

@stof Thanks for the comment. Good points. The one thing about passing the
data explicitly is that it feels a little heavy and not so DRY. As I
understand it, I would have to subclass Event to create an event that
stores extra data. And then I would either have to explicitly call "new
EventWithData($this)" in my app code when dispatching (coupling my code to
this new event class) or subclass EventDispatcher to create EventWithData
instances instead of Event instances. I don't like either of these
approaches very much. Maybe there is another way I am overlooking?

I feel like the Event class could really use at least a simple way to
attach user data -- e.g.: trivial setData and getData methods. That would
seem to eliminate a lot of need for subclassing.

Marc

@stof
Copy link
Member

stof commented Apr 4, 2012

@msabramo setData and getData does not provide a clean OO api to the listeners. what are the data ? a string, an array, anything the original code wanted ? This is even worse when you have a setter (what if the first listener replaces a string by an array and the second listener was expecting to receive a string ?). Subclassing the event allows to provide a clean api in your class (having getRequest, getKernel and setResponse for instance in GetResponseEvent when you look at the HttpKernel component)

@msabramo
Copy link
Author

msabramo commented Apr 4, 2012

@stof Yeah, I get your point. I will sometimes compromise on OO and stricter typing to get more flexibility. This is part of what I like about PHP and Python vs. Java and C++. The data in this case would be defined by the calling code and EventDispatcher would just be passing it along without doing any interpretation. It does encourage a looser style that perhaps is not the Symfony way though so I can understand not wanting to do that. Maybe I just need to get over my aversion to subclassing :-)

And thank you for mentioning the HttpKernel example. I'll be taking a look at that shortly.

For the sake of completeness, I'll post a link to an idea that Drak put forth, because it's similar. I don't think this is any better, but it might be interesting for some future reader to get more of an idea of what we're talking about.

https://github.com/drak/symfony/compare/genericevent

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.

2 participants