On 28.01.2008 11:52:23 Vincent Hennebert wrote:
> Jeremias Maerki wrote:
> > On 27.01.2008 16:15:45 Vincent Hennebert wrote:
<snip/>
> 
> <snip/>
> >>> No, the exception should always contain the cause.
> >> Then I’m missing the point of the new feedback mechanism. Why would you
> >> have to both create an event and throw an exception? All the more than
> >> the exception’s message wouldn’t be localized. And in the case of e.g.
> >> an error in the FO file we all but care about knowing at each place of
> >> the code exactly did the problem occur. Why couldn’t the broadcaster
> >> stop the process?
> > 
> > Because a broadcaster (as its name says) only delivers messages.
> > Interrupting the process flow is the job of the programming language's
> > exception feature. Again, the event flow is mainly for informing the
> > user (or the technical user). He may or may not react on it. An
> > exception happens on exclusively a technical level (and in the past
> > could bubble up to the user because there was nothing else). Of course,
> > you could just swallow the last event that is generated right before the
> > exception but then you have to make sure the exception can equally be
> > localized, contains the same information as the event and you have two
> > different sources for event information (which the user has to know).
> 
> Fine. I still disagree with that, but I understand your point about 
> “only delivers messages”, and in the end it’s all a matter of how this 
> will be implemented. So I’ll get back to you once I’ve had a look at the 
> code, if I ever feel the need to. I just expect the exception to be 
> caught by a higher-level object and transformed into an event like “A 
> fatal error occured, aborting.”. Because, let me just repeat this, 
> I don’t think the stacktrace is of any interest to the user, in case of 
> a user-level error.

Ah, that's what you meant. Of course, I agree to that. Seems I
misunderstood you.

> Only when there’s a programming error in the code 
> should the stacktrace be revealed, which will happen independently of 
> the feedback mechanism anyway, if I’m correct.

Yes.

> 
> > The main point of the feedback mechanism is to have a sequence of
> > notifications about the events in FOP (per processed document) so an
> > application can react on them and the user is informed about what's
> > going on. An exception is after all only a one time information and may
> > not happen at all (even though there may be multiple events during a
> > processing run). At the moment these events only appear in the log in an
> > unstructred, non-machine-readable way and without the benefit of keeping
> > the events appart in a multi-threaded system.
> > 
> > I acknowledge that there is a certain overlap area between an exception
> > and the last event that indicates a fatal error. But sending the fatal
> > error event just before throwing the exception just makes sure we have a
> > uniform way of sending notifications to (potentially) multiple information
> > sinks. The "fatal error" will always produce an exception afterwards so
> > the integrator will know that he doesn't have to translate the exception
> > into a form that he can display it in the same spot as the events. If he
> > wants to filter the fatal exception, he can still do that, of course.
> > Again, I consider exceptions and events two completely different things
> > even if at first sight, it looks like redundancy.
> > 
> <snip/>
> >>>> To keep on the safe way you would
> >>>> probably have to do something like
> >>>>     public BasicFOValidationEventProducer 
> >>>> getBasicFOValidationEventProducer()
> >>>> and add as many methods as there are EventProducer implementations. Just
> >>>> forget that and go with the simple version.
> >>> -1 to that because
> >> Yeah, that’s precisely what I’m saying.
> > 
> > Beg your pardon?
> 
> See above: “Just forget that and go with the simple version”.

Ok. Anyway, just for illustration I've added such a wrapper. One can
easily see in the diff what change that causes to the client code:
http://svn.apache.org/viewvc?rev=615773&view=rev

> >>> I will import interfaces from all sorts of packages
> >>> in FOP, at least if these methods are added to DefaultEventBroadcaster.
> >>> Better would be something like:
> >>>
> >>> public class BasicFOValidationEventProducerFactory {
> >>>   public static BasicFOValidationEventProducer 
> >>> getBasicFOValidationEventProducer(EventBroadcaster broadcaster) {
> >>>     return 
> >>> (BasicFOValidationEventProducer)broadcaster.getEventProducerFor(BasicFOValidationEventProducer.class);
> >> <snip/>
> > 
> > No comments on the counterproposal?
> 
> Since you’re asking ;-)
> I think it’s equivalent (may be wrong, though). In my version the 
> generic method would be private and called by the typesafe ones:
>     return (BasicFOValidationEventProducer) 
> this.getEventProducerFor(BasicFOValidationEventProducer.class)

IMO, it's not equivalent. See the reason for my -1: the various imports
of all sorts of packages where the event interfaces will be located. The
"events" package is designed to be a tool package and therefore should 
(theoretically) be useful outside of FOP (once some of the names are
changed). For me part of a good design is: design something for
usefulness in various contexts and add adapter code specific to an
application elsewhere. Otherwise, the package is immediately restricted
for use in one single application.

> In your version I guess it would be package-private and the 
> *EventProducerFactory classes would be defined in the fop.event package. 

No, they would be located in the contextually appropriate package. With
the implementation above I've actually implemented the Factory as a
nested class in the interface which makes this slightly more elegant
(IMO).

The Ant task I've built searches the whole source directory of FOP for
such EventProducer interfaces. It doesn't really matter where they are
defined, so I'd like to put them where they make most sense.

> Mine is lighter, yours is more flexible because it’s easy to add new 
> factories. And probably cleaner in the long term if lots of different 
> factories need to be defined.
> But in the end the simple currently-type-unsafe-but-will-soon-be-with-
> Java-1.5 is just fine.

Ok, we'll see how that works out.


Jeremias Maerki

Reply via email to