I read with fascination Jeremy’s “Braindump on the Event Aggregator Pattern” and recommend it to you, gentle reader. You’ll find concise coverage of the intent and the issues that confront anyone who would build his/her own EA.
Half way in Jeremy launches a critique of EA as implemented in Prism. It reminded me of my initial reaction to Prism's EA which was "man, this seems clunky." Ouch!
One forgets in time and just accepts all the extra motion as "just the way it is". After awhile you don't even see it anymore. Then someone – a Jeremy – comes along and wags his finger at it.
Now I like Prism's reliance on strongly typed EA events. Much better than the string “Event Topics” of CAB. But, he is right. I shouldn't have to publish an event by writing:
// Publish an event with no payload (have to fake it); the event type is the message
_eventAggregator.GetEvent<CacheClearEvent>.Publish(null);
// Publish an event with a strongly typed payload
_eventAggregator.GetEvent<EntityNotificationEvent>().Publish(eventPayload);
Aside: I don't mind defining strongly-typed CacheClearEvent and EntityNotificationEvent. But Prism forces me to define an EventNotificationEventPayload class to support the second event. This smelled wrong ... but I persevered.
Nor should I have to subscribe by writing:
_eventAggregator.GetEvent<CacheClearEvent>().Subscribe(Clear);
_eventAggregator.GetEvent<EntityNotificationEvent>().Subscribe(SetCustomer);
The issue here is that it takes two steps to publish and subscribe. First I have to get the event object from the EA and then I tell it what I want to do. This bugs Jeremy. And now it bugs me.
My initial reaction was "let's just clean this up with some extension methods."
I set aside, for the moment, his desire to eliminate the subscription line altogether; one step at a time, OK?
Ah ... but as soon as I started working on those extensions, I realized why the Prism team hadn't done this themselves. I saw how the team got hung up.
The Prism EA designers made a fundamental decision that the strongly-typed event must have a separate, strongly-typed payload. You see this in the signature of the base class for all such events, CompositePresentationEvent<TPayload>. If you want to define a pub/sub event in Prism you must inherit from this class.
This leads you to the following two extension method signatures:
public static void Subscribe<TEventType, TPayload>(
this IEventAggregator aggregator, Action<TPayload> action)
where TEventType : CompositePresentationEvent<TPayload>
public static void Publish<TEventType,TPayload>(
this IEventAggregator aggregator, TPayload payload)
where TEventType : CompositePresentationEvent<TPayload>
These seem ok until you try to use them. You end up with this:
_eventAggregator.Publish<CacheClearEvent, object>(null);
_eventAggregator.Publish<EntityNotificationEvent, EntityNotificationEventPayload>(eventPayload);
_eventAggregator.Subscribe<CacheClearEvent, object>(null);
_eventAggregator.Subscribe<EntityNotificationEvent, EntityNotificationEventPayload>(SetCustomer);
I’m not sure I’ve gained much clarity. Those type parameters are plain ugly.
While I can add more extension methods to smooth the way for the events that take no payload (e.g., CacheClearEvent), I'm stuck with this syntax for the more interesting events that take a payload. Maybe you can finesse them away; I can’t find a way.
This lead me to ask "what if an event that needed a payload was itself the payload?" I realized I could bring this off with the existing Prism EA ... if I adopted a rather strange convention.
Here is my new CacheClearMessage for example.
public class CacheClearMessage : CompositePresentationEvent<CacheClearMessage> { }
Notice how it inherits from CompositePresentationEvent<TPayload> as required. But it cleverly references itself as the payload.
My new EntityNotificationMessage looks like this:
public class EntityNotificationMessage
: CompositePresentationEvent<EntityNotificationMessage> {
public EntityNotificationMessage() {}
public EntityNotificationMessage(Type entityType, object entityId) {
EntityType = entityType;
EntityId = entityId;
}
public object EntityId { get; private set; }
public Type EntityType { get; private set; }
// more stuff
}
Notice that it contains its own payload which happens to be info about the subject of the entity notification. I no longer need my EntityNotificationEventPayload class which I delete from my project (yipee!)
Notice I can instantiate the message without a payload too. Prism requires a parameterless ctor in order to register the event; you wouldn't actually use this one.
Now I add two extension methods that look like this:
public static void Publish<TMessageType>(
this IEventAggregator aggregator, TMessageType message)
where TMessageType : CompositePresentationEvent<TMessageType>
public static void Subscribe<TMessageType>(
this IEventAggregator aggregator, Action<TMessageType> action)
where TMessageType : CompositePresentationEvent<TMessageType>
My client code looks much cleaner now:
_eventAggregator.Publish<CacheClearMessage>();
_eventAggregator.Publish(entityNotificationMessage);
_eventAggregator.Subscribe<CacheClearMessage>(Clear);
_eventAggregator.Subscribe<EntityNotificationMessage>(SetCustomer);
I get type inference in only one usage (one of the Publish calls); maybe the explicitness is not so bad. At least there is only one type parameter.
--
I'm not as freaked out by the subscriptions but I get Jeremy's point. I should be able to identify the Clear and SetCustomer methods as methods to be subscribed to. I shouldn't have to explicitly import the EventAggregator and wire the methods to it.
I'm not sure what the best way to get around that is just yet (and he doesn't seem to have settled on an answer either). So I'll just stop right here for now.
Tonight, as I post, I'm feeling that this was a good refinement. I see no benefit in forcing separation of the event class and the payload class. Maybe the Prism designers will educate me. Maybe I'll wake up tomorrow with a hangover and wish I'd left well enough alone.
Your thoughts are welcome.
13 comments:
Six months to a year ago, I would have had very little opinion as to how the API for an event aggregrator should look. In that time, I've been studying distributed application architectures, which led me to investigate OSS projects like NServiceBus, Rhino ESB and MassTransit. What's interesting is that these frameworks are doing the same, conceptually as the event aggregator. I think a lot could be learned from studying what they do and why. They tend to have some pretty clever mechanisms for pub/sub. In the end, the public API looks much like your improved versions, Ward. All you should have to do is something like: _bus.Send(message) or _bus.Subscribe<SomeMessage>(this).
Thanks, fellas.
@Glenn - Yes .. it IS A HACK. No question.
I considered creating a new derivative of EventBase, called MessageEvent. Prism's EA let's you do that kind of thing.
Unfortunately, I would have duplicated the code inside CompositePresentationEvent (what a name!). This did not appeal to me last night and looks no better this morning.
I wish I had entitled this post "The Medium is the Message" after Marshall McLuhan which would have captured the essence of my revision.
In Prism, the event and the payload are separate ... which makes for ugly C#. My little trick combines event and payload: the medium is the message.
Maybe the GetEvent<> pattern has utility I don't yet recognize. No longer bothers me when I can shield with extension methods.
@Rob - Right on. You remind me that I should have put the punch line at the top instead of making you wade to the end for it. Not thinking clearly at 1:30am (if ever).
Clever, very clever, though my friend it feel like hack :-)
As far keeping the event separate, we EA is not the first to do that, as the estabilished pattern in the framework is that args are separate from the event.
FWIW, I thought the GetEvent<> pattern was weird the first time I saw it, and I remember raising this to team. We talked about it and there were several reasons why we ended up on this approach.
On of these rules was the rule that I had set which enforced agreement in signature between the publisher and subscriber.
Having an Event< T > means the Publish and Subscribe methods can enforce the correct payload.
Jeremy and I have had several chats in the past and he has expressed his distaste with the approach. I do tend to like the IListener < T > approach better as it is cleaner. It approaches more of the syntax that Rob is looking for.
To implement, you apply an IListener< T > interface which the subscriber implements. Then the subscribe method can be implemented as void Subscribe< T > (IListener< T > listener). Or as in Jeremy's case you can autowire up through StructureMap :-)
IListener< T > looks like this,
public interface IListener< T > {
Execute(T message)
}
so for example my OrderDetail might subscribe to the OrderSelected message...
public class OrderDetail : IListener< OrderSelected > {
public void Execute(OrderSelected message) {...}
}
Thus you still get the strong typing as the listener must implement the correct interface.
What I like about this approach is the fact that it can be autowired externally, and unlike CAB there's no attributes required as you infer from the interface.
All that being said, I don't think the pattern in Prism is bad, and it seems like a bit of minutae that we are talking about.
That doesn't mean it can't be improved, there is always room for more refactoring :-)
@glenn Thanks for the elaboration.
I understand that the established pattern is to separate events from args. I'm just quarreling with the establishment ... 'cause I'm a 60's guy.
The message approach I broached is every bit as strongly typed as Prism's. Remember that in this message, the "event" IS the "payload". The message is strongly typed and, therefore, so is the payload as is clear from the signature, Publish<TMessage>(TMessage msg)
It is just as clear that subscriber must type match the publisher: Subscribe<TMessage>(Action<Message> action).
What am I missing in this regard? How can the payloads be mismatched? How is there better enforcement with a separate TPayload?
Autowiring is just as feasible because TMessage inherits from a base class that IoC can detect. Nothing prevents my IoC from autosubscribing any method with a signature conforming approximately to
Action<Action<TMessageEvent>>.
"Look, ma ... no attributes". No interface needed either (although go ahead and provide a marker interface to go with the MessageEvent base class if you want).
p.s.: "MessageEvent" is my shorthand for the message form of that CompositePresentionEvent<TPayload> thingy.
I am not freaked out by Prism's EA as it is today. I think it could be cleaner and simpler and lend itself to autowiring and all of that would be goodness.
I also demonstrated that I can take it that direction all by myself w/o modifying Prism or asking anyone else to do so. I think it would be better if Prism added the MessageEvent and the extension methods. But it isn't essential.
As Jeremy noted, one of the nice things about Prism is that, if you don't like something, it's easy to fix or replace.
I think it's a matter of preference. Our preference was stickin with something that more closely mimicked the .NET Eventing model, as we felt it made it more familiar. Having T be a message that includes the parameters, certainly seems reasonable
though, and also fits better with the message-based architects that are becoming so prevalent.
The other difference in terms of approach comes around the autowiring. IListener< T > really shines when you have an IoC behind the scenes which automatically finds IListener< T > implementations and wires them up. Prism's EA however is a more explicit model, where the subscribe actively subscribes. This has benefits in terms of debugability, as well as making the code less "magical", are removes a dependency on something doing the autowiring.
I will say the more that I think about it, IListener< T > is easier to test as I simply cast the received to the appropriate interface and invoke the interface in a UT.
That's "Message based architecture" not "architects" :-)
Sorry to interject: The Curiously-recurring Template Pattern (CRTP) is no hack. It is so beautiful and useful. The fact that you HAVE to rely on it in order to work around some pain may be a bit hacky, but using it is not.
Have you guys not read about it? It's an older pattern - I think I first read it in a Herb Sutter article, or maybe Alexandrescu's Modern C++ Design. Here's a link to the wiki page: http://en.wikipedia.org/wiki/Curiously_recurring_template_pattern
@Kyle P. - Re: CRTP, thanks for demonstrating that there is a name for almost everything :-)
Let's not disagree about whether it is a hack to use this pattern.
I meant to make the following point: it is unfortunate that one cannot immediately appreciate by reading this code what the thing is, what it does, and why I'm doing it that way.
Those who know the idiom have a headstart ... but they still don't know why I'm doing it.
It would be less distracting and more straightforward if Prism offered out-of-the-box something like an EAMessageEvent type (not a good name I hasten to add).
Heh, indeed. I certainly agree with the bulk of your post, of course. :)
Hey Ward
I took another shot at the syntax and came up with an alternative approach to remove the GetEvent<> pattern through some extension methods. This one doesn't require any "special" event / message classes.
Posted here (blog post pending): http://codepaste.net/uf9isv
Hello Ward and thanks for your post.
Is there a way to make other extensions so that I don't have to pass a message at all (and the corresponding Action will be parameterless)?
Because really there are some events that don't need parameters, e.g. UserLoggedOutEvent.
In such a scenario the event type is only a body class that can even be sealed with a private ctor.
So I am looking for some kind of extension that will serve the following (of course, the problem is handling the Unsubscribe that it should also take an 'Action').
eventAggregator.Subscribe(this.UserLoggedOut);
//...
this.eventAggregator.Publish();
//...
this.eventAggregator.Unsubscribe(UserLoggedOut);
//...
void UserLoggedOut()
{
//Do action,
//I don't care who sent this msg
//Nor do I need any params
}
Would appreciate any response
@Shimmy. What message are you publishing when you write: "this.eventAggregator.Publish();"?
See the problem? The type provides both the message-type and the message-payload. You don't have to have a payload; you do need something to convey the message-type.
I don't think you want whatever type "this" is to be the message-type.
Maybe I misunderstand your question.
isn't the problem with this that the event will persist the 'payload' content..meaning each time you call eventAggregator.GetEvent(), the you are getting a single instance of any object. in a multithreaded messaging environment, isn't this problematic?
Post a Comment