[ALACPP] Thoughts on last night's code sample

[Christopher Smith <x@xxxxxxxx>]

So, after integrating the abomination that is Boost PP into the already
hideously complex organic parser in my cranium. I was able to step back
from the shock and awe at the code (btw, where is the electronic
version) and come up with what I hope is constructive criticism.

Criticism 1: The terminology seems wrong to me.

This is perhaps a personal pet peeve. I tend to think of event systems
as having a few distinct components: the event (surprise), the event
source, the event dispatcher (sometimes this is effectively invisible),
and the event handler. Based on my own glossary CEventHandler looks like
an event dispatcher more than event handler. It is essentially a level
of indirection away from the event handlers, which appear to me to be
the function(s) contained by the CEventHandler.

Criticism 2: The synchronization model is a tad poor.

The code's current synchronization model is to acquire a mutex, iterate
through and invoke all all the functions in turn, then release the
mutex. This is bad for two reasons: you hold the lock for a
non-deterministic (and potentially long) time and handlers can have side
effects on event dispatch to subsequent handlers. What you normally want
to do is acquire the mutex, get a *copy* of the function list, release
the mutex, and then iterate over the copy and invoke the functions.

I don't think this was a problem for the particular implementation
because the handlers were primarily serializing the events, and then
firing them up, so event dispatch would primarily be about marshalling
events into a socket buffer, and the handlers tended not to effect each
other.

Criticism 3: Use event objects, rather than argument lists.

The more I think of it, while all the Boost PP foo is very cool,
ultimately it makes sense to take the standard approach of templating on
a single parameter, which is an event object. Not only does this give
you a cleaner object model (and avoid the need for Boost PP I think), it
avoids of host of problems. The penalty for this is that you have to
define a lot of event classes, and are unlikely to find an existing off
the shelf-function which takes an event object as a parameter. For the
particular problem domain you are working on, I'd say that event object
would provide a really nice insertion point for a serialization
framework (and for that matter probably a number of other frameworks I'm
sure).

For the latter, I'd argue that it's unlikely under any circumstances
you'd have a function that matches up against your "event arglist", and
for those cases that you do it'd likely because the "event arglist" was
shaped by the function's argument list (which of course kills the loose
coupling that the event system is supposed to provide). More importantly
though, it avoids some potential semantic ambiguity with the arglist.
So, for a particular event you might have two parameters, say two
strings. You could have another event, with an entirely different
meaning and context, but which also takes two strings as it's
parameters. Since they would both have the same argument list, they
would both trigger the same handlers. The only way to do this would be
to artificially introduce a 3rd parameter which served to disambiguate
the two. Instead, just use a different event class for the two events,
and the ambiguity goes away.

Criticism 4: No need to have assignment policies.

I'll concede that maybe I missed something here, but the assignment
policies seems quite limited to me. By simply adding a
"setFunction(_Fnt)" method to the multi-assignment model (which would
clear the current function list and make the function the only member of
the list. If you need to enforce at compile time that there can be only
one function, you can use a static assertion, but this seems like an
unlikely need. The current model, while specializing for the single
handler case, still uses an iterator when doing event dispatch. At best
you're saving a few bytes of storage in exchange for a not insignificant
increase in complexity.

Criticism 5: I hate the brace layout style. It's unhealthy for my brain.

Sorry, couldn't resist. ;-)


Anyway, I'd love to get feedback from everyone else on these points.
It's always cool when you have some actually code to talk around. :-)

-- 
Christopher Smith <x@xxxxxxxx>
_______________________________________________
alacpp mailing list
alacpp@xxxxxxxxxxx
http://lists.ellipsis.cx/mailman/listinfo/alacpp