Re: [ALACPP] Thoughts on last night's code sample

["Jon Stewart" <stew1@xxxxxxxxxxx>]

> 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.


The electronic version is at http://www.alacpp.org/eventhandler.zip; it's 
linked to from the main page.


> 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.


This is a fair criticism. We used it as a dispatch; we weren't much good 
at naming things. The code itself is decently general, a customized 
functor.


> 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.


Yes. And the SingleEventHandler specialization was the dominant type.


> 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.


True, but. Sometimes arguments can be lumped together into an actual
object (one that has some behavior) and sometimes they can only be lumped
together into a struct. And, damn, creating dummy structs is annoying,
with or without boost::tuple. In our networking framework, each "type" of
connection got a separate class; within those classes, the number of
available messages to send and receive was quite small, and argument
duplicates were quite rare. Not having to define a separate class for each
message was a definite win -- the very best part of our network library
was the minimum of busy work necessary to create a new network message.
Because of that leverage, we were able to get an incredible amount of work
done in short order (although any time we saved was more than offset by
the time put into slapping bandaids on the lower levels of our networking
framework, but that's another story...).


> 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.


You're right.


> Criticism 5: I hate the brace layout style. It's unhealthy for my brain.
> 
> Sorry, couldn't resist. ;-)


I actually need to give begrudging props to the old Silver Platter coding 
standard. Why? Adapting to any other coding standard, however insane, is 
considerably easier for me to do now.

That said, I do still hate the brace layout style. The original 
inspiration was Python. Python to C++ is a non-sequitur.



Jon
-- 
Jon Stewart
stew1@xxxxxxxxxxx
_______________________________________________
alacpp mailing list
alacpp@xxxxxxxxxxx
http://lists.ellipsis.cx/mailman/listinfo/alacpp