Hi, On Wed, Jan 29, 2014 at 4:44 AM, Michael Dürig <[email protected]> wrote: > I don't like how this relies on side effects: generate() puts the events > into a queue owned by QueueingHandler and referenced by this class > (EventQueue).
Those aren't side effects, they're the explicitly what the classes are wired up to do. There's no other effects of that wiring. The responsibility of the EventGenerator is to incrementally run through the content diff, making callbacks to the given handler. The responsibility of the QueueingHandler is to turn those callbacks to JCR events and place them in the given queue. The responsibility of the EventQueue is to manage the queue and make it accessible to higher level clients. > Look how concise this was initially: > > concat(listener.iterator(), concat(childEvents.iterator())); > > just flatmap that shit ;-) Right, but doing so caused OAK-1318 and ended up requiring a custom iterator wrapper that worked much like EventQueue now does. > Also EventQueue is not really a queue but just an iterator implementing that > flatmap by means of the generator and the queueing handler. Could we at > least somehow remove that explicit reference to the queue from EventQueue > and access it through the queueing handler? That's something I wanted to avoid, see the list of responsibilities above. To make this more explicit, we could replace the direct LinkedList accesses in QueueingHandler with something like EventQueue.addEvent(), which would encapsulate the queue entirely within EventQueue. BR, Jukka Zitting
